Reduce object creation in parsing#6750
Conversation
kurkle
left a comment
There was a problem hiding this comment.
You'd need to add the array maintenance done to meta.data when inserting / deleting / resyncing elements to this new array.
I used _val in doughnut to make it clear that it's not something done by DatasetController._parse() and does not work the same way (no axis.id). But that change is fine by me.
I'd actually like to change the doughnut to use an scale to be consistent (I think some issues could be solved that way too, but am too lazy to look for them now)
What if we left it under meta.data, but instead of replacing it, just set the values in it? Wouldn't it be more efficient that way (removing the extra array maintenance). If going that way, then those meta creation functions cant be replaced.
We could maybe reduce the memory footprint by placing the parsed data directly to meta properties, instead of under object. Would need to prefix the variables somehow, to try avoiding collisions (like meta['_parsed'+axis.id]). Not sure if that would be more efficient compared to meta._parsed[axis.id]
| return type && new type({ | ||
| _ctx: me.chart.ctx, | ||
| _parsed: {} | ||
| _ctx: me.chart.ctx |
There was a problem hiding this comment.
Looks like createMetaDataset and createMetaData could be replaced with createMeta(type)
I like this too. Same with bubble chart radius which is missing a scale. |
|
Closed in favor of #6758 |
There was a line (
_parsed: {}) that created a new object for each index, which I think caused extra garbage collection since that object was never used and ended up getting replacedThere seemed to be a bug in
insertElements.addElementsAndResetcalledupdateElementwhich depended on parsing already having happened. It seems like it was able to complete before because it had the empty object to reference, but it was uncovered when I got rid of thatI'm not sure why doughnut was using
_valinstead of_parsed. This changes that for consistency