-
Notifications
You must be signed in to change notification settings - Fork 15
CPU and Memory Optimizations #56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CPU and Memory Optimizations #56
Conversation
|
This looks great 👍 |
5e4b971 to
02f53c4
Compare
|
@iabmayank @chuff can you please take a look at this? |
8bac901 to
c30373e
Compare
|
@iabmayank @chuff i have rebased off of the most recent master |
|
@yuzawa-san thank you for submitting this, we will be reviewing these in the working group |
|
@yuzawa-san |
|
@chuff removed i would like to try adding it again in the future in a separate pr since i feel it is quite important that contributors have the ability to easily generate benchmarks. just curious, why did you favor removing it? |
|
@chuff @AntoxaAntoxic @ChristopherWirt this is now ready for review. i have added some more changes to this pr. i am coordinating with the maintainers @lamrowena and this PR is allowed to have breaking changes, so i have gone forward with the more aspirational (yet breaking) changes i had mentioned in the original pr description. preliminary benchmarks show its even faster than the prior benchmarks, so i'll try to get those up soon. |
98.7% less memory (which means less GC activity, which means less CPU) |
897eb42 to
5b10385
Compare
optimize fibonacci encoding clean up overflow validation optimize fibonacci end tag finding cache fibonacci numbers and enforce limit
623ee38 to
8a87187
Compare
I found the CPU and Memory in the decode hot path is very high, so I did some light refactoring to alleviate this.
Made FixedBitfieldEncoder return BitString directly which does fulfillFixed bit ranges are backed by bitsets.List<Boolean>. This is a lot smaller that theArrayList<Boolean>+usages since that is optimized (in JDK8) to a StringBuilder. if it used the char constant it appears to have to convert those chars to Strings each time. however in later JDK's this is not needed.NOTE: the encode flow could technically use the BitString too, but i held off on that for now.the encode accumulates using more efficiently. toString is only called at the last minute.microbenchmark results:
seems to be around 6x faster than the last released version and uses only about 97% less memory.
ad-hoc benchmark code against https://github.com/InteractiveAdvertisingBureau/iabtcf-java (partially ported into JMH)
benchmarked using async-profiler
asprof -d 20 -e cpu,alloc -f ~/Desktop/dump16.jfr TcfBenchFlame Charts:
memory before:

memory after (note how TCString parse was small teal sliver in the before graph, but the iab-gpp portion has shrank so much that the TCString parse is now a larger percentage of the icicle chart):

cpu before:

cpu after:

FutureAdditional Ideas(not in PR). I'll likely open issues to discuss:since this a beta candidate i have broken compatibility on this and introduce an IntegerSetList<Integer>is still a little bulky in the charts above. I was thinking of maybe making a specialty class backed by int[] like https://github.com/InteractiveAdvertisingBureau/iabtcf-java/blob/master/iabtcf-decoder/src/main/java/com/iabtcf/utils/IntIterable.java but that would likely break API stabilityShould the fields map have keys be enum's instead of strings? The EnumMap would be a lot more lightweight.the Fields are stored in a more efficient manner using a listshould List be replaced with Set since I feel set contains is something worth optimizing (i.e. is this vendor(s) present in the set)once, again IntegerSet extends AbstractSetare the defensive copies in some getValue implementation necessary, could we achieve the same from returning read-only versions (Collections.unmodifiableList).introduced a managed IntegerSet which can detect modification and update the dirty flags, such that once could do tcfeuv2.getPurposes().addInt(10)fixes #25
supersedes #45