Open shappir opened 9 months ago
This is the reason Object.freeze
needs to be removed: https://github.com/tc39/how-we-work/blob/main/terminology.md#override-mistake
Do you have any umbers or graph to confirm this helps things?
No systematic results. I will try get some.
My tests show a memory saving of only 5% 😢 Guess I shouldn't have called it significant ... It's borderline worth it - your call. (I will add the test example into the repo if you want.)
How many buckets total, and how many with values, did you test and get 5% savings?
@zbjornson @SimenB I tested one histogram with the default buckets: [0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10]
Tests:
I got roughly the same results in all cases, peaking at a saving of 5%.
Generally speaking (in percentages):
Significantly reduce the amount of memory used by histograms, especially for high cardinality/lots of buckets:
bucketValues
object with prototype of zero values instead of copying. This way counter per bucket is only allocated when its value is greater than zero (first time it's incremented).bucketExemplars
(when using exemplars) instead of pre-filling with nulls.Additional optimizations:
valueFromMap
into hash only when it's allocated (don't reinsert every time)bucketExemplars
. Instead reuse previous lookupNotes:
Object.freeze(this.bucketValues)
because it causes += 1 to fail, even though the value in the prototype isn't actually changed. (This feels like a JS or v8 bug)hasOwnProperty
to check for bucket existence