Skip to content

Conversation

@ngokevin
Copy link
Member

@ngokevin ngokevin commented Aug 9, 2017

Description:

Rather than create a new object each time to create a new oldData, just have two objects representing oldData and data. Also when updating, rather than create a new data object, have a newData object that is used for holding the new data which then gets copied into data. So in total, only use three objects: oldData, data, newData.

Changes proposed:

  • Adapt style-attr implementation to be able to pass in an object for reuse instead of creating a new one each time. Also remove lots of callbacks, Object.keys() array creations from their code. Also tweaked to do prettier stringifies. We manage this forked dependency now.
  • Use object pooling in core component code to reduce object allocations. Recycle them when component is removed.
      1. Remove object allocation on each update for creating object for oldData.
      1. Remove object allocation on initialization for initial oldData.
      1. Remove object allocation on each update during buildData for default value copying.
  • Add some helper properties to component (isSingleProp, isSinglePropObject, isObjectBased)
  • Update our styleParser utils a bit to not allocate object + array + use callbacks to convert keys to camelCase.

var schema = this.schema;
if (typeof data === 'string') { return data; }
if (isSingleProp(schema)) { return stringifyProperty(data, schema); }
if (this.isSingleProp) { return stringifyProperty(data, schema); }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Full name isSinglePropertySchema?

@ngokevin
Copy link
Member Author

Spent a day to get this updated with passing tests. Optimizing the component core, string parsing, and adding in object pooling practices.

@ngokevin
Copy link
Member Author

ngokevin commented Oct 2, 2017

@dmarcos r?

@ngokevin ngokevin force-pushed the objectpool branch 3 times, most recently from a5a92e2 to 9281409 Compare October 3, 2017 17:18
@ngokevin
Copy link
Member Author

ngokevin commented Oct 3, 2017

Updated to not delete the keys when recycling an object, but just set every key to undefined. This means object pooling best used when objects share a common schema, in case we iterate over keys. So I create different object pools for each type of component, since their object keys will be the same. I also update schema parser to ignore undefined keys.

Before

screen shot 2017-10-03 at 7 16 43 pm

After

Improvements: See the updateProperties and updateCachedAttrValue no longer on the list of frequent memory allocators. (I have other PRs open to remove deepEqual and updateControllerList from that list).

screen shot 2017-10-03 at 7 17 57 pm

@ngokevin
Copy link
Member Author

Updated. Still need to polish/comment a bit more (and try to remove the clones for default values). But up and working again.

@dmarcos
Copy link
Member

dmarcos commented Nov 2, 2017

Tests fail

@ngokevin
Copy link
Member Author

Moved to #3772

@ngokevin ngokevin closed this Sep 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants