So... if we essentially copy JSC's optimization called by Jeff in Comment 1, we can get a lot faster... ``` Benchmark 1: /home/matthew/unified/obj-opt-shell-nodebug-x86_64-pc-linux-gnu/dist/bin/js keys-benchmark.js Time (mean ± σ): 422.8 ms ± 1.3 ms [User: 339.0 ms, System: 95.0 ms] Range (min … max): 421.6 ms … 425.9 ms 10 runs Benchmark 2: ENABLED=1 /home/matthew/unified/obj-opt-shell-nodebug-x86_64-pc-linux-gnu/dist/bin/js keys-benchmark.js Time (mean ± σ): 57.0 ms ± 0.3 ms [User: 53.9 ms, System: 4.3 ms] Range (min … max): 56.3 ms … 57.9 ms 51 runs Summary 'ENABLED=1 /home/matthew/unified/obj-opt-shell-nodebug-x86_64-pc-linux-gnu/dist/bin/js keys-benchmark.js' ran 7.41 ± 0.05 times faster than '/home/matthew/unified/obj-opt-shell-nodebug-x86_64-pc-linux-gnu/dist/bin/js keys-benchmark.js'` ``` This patch is -extremely- not production quality, but the general gist is that it rides on the already existing ShapeCachePtr and stores the previously computed set of properties, and births a new array from these values at a cache hit (this suggests that array allocation isn't ultimately the long pole here as well). If we wanted to productize the patch, there's a bunch of work we'd need to do 1. Adding a new cache type to the ShapePtrCache means that when one cache is active, others don't attach. This conflict likely needs to be resolved in some fashion. If I modify my benchmark to bring back the 'for-in' activating loop mentioned in Comment 7, now this patch provides zero speedup, because we can't attach the cache, since the iterator cache is occupying those bits. (Perhaps the ShapePtrCache needs to instead key into a data table -- IIRC that was JSC's approach) 2. This would need some thorough evaluation from performance and memory usage. The above makes it likely very easy to get this wrong. The flush-cache-on-gc thing seems sensible tho, and would limit the potential harms a bit. 3. The code itself is definitely not production quality (yee-haw mallocing and almost certainly leaking memory).
Bug 1836679 Comment 9 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
So... if we essentially copy JSC's optimization called by Jeff in Comment 1, we can get a lot faster... ``` Benchmark 1: /home/matthew/unified/obj-opt-shell-nodebug-x86_64-pc-linux-gnu/dist/bin/js keys-benchmark.js Time (mean ± σ): 422.8 ms ± 1.3 ms [User: 339.0 ms, System: 95.0 ms] Range (min … max): 421.6 ms … 425.9 ms 10 runs Benchmark 2: ENABLED=1 /home/matthew/unified/obj-opt-shell-nodebug-x86_64-pc-linux-gnu/dist/bin/js keys-benchmark.js Time (mean ± σ): 57.0 ms ± 0.3 ms [User: 53.9 ms, System: 4.3 ms] Range (min … max): 56.3 ms … 57.9 ms 51 runs Summary 'ENABLED=1 /home/matthew/unified/obj-opt-shell-nodebug-x86_64-pc-linux-gnu/dist/bin/js keys-benchmark.js' ran 7.41 ± 0.05 times faster than '/home/matthew/unified/obj-opt-shell-nodebug-x86_64-pc-linux-gnu/dist/bin/js keys-benchmark.js'` ``` This patch is -extremely- not production quality, but the general gist is that it rides on the already existing ShapeCachePtr and stores the previously computed set of properties, and births a new array from these values at a cache hit (this suggests that array allocation isn't ultimately the long pole here as well). If we wanted to productize the patch, there's a bunch of work we'd need to do 1. Adding a new cache type to the ShapePtrCache means that when one cache is active, others don't attach. This conflict likely needs to be resolved in some fashion. If I modify my benchmark to bring back the 'for-in' activating loop mentioned in Comment 7, now this patch provides zero speedup, because we can't attach the cache, since the iterator cache is occupying those bits. (Perhaps the ShapePtrCache needs to instead key into a data table -- IIRC that was JSC's approach) 2. This would need some thorough evaluation from performance and memory usage. The above makes it likely very easy to get this wrong. The flush-cache-on-gc thing seems sensible tho, and would limit the potential harms a bit. 3. The code itself is definitely not production quality (yee-haw mallocing and almost certainly leaking memory). Edit: oh and I forgot. most importantly, this needs to be made sound! We need to figure out how to handle indexed properties being added and ensure we flush the cache when that happens.