Avoid expensive for..in loops involving CMaps #5101
Conversation
| }, | ||
|
|
||
| contains: function(code) { | ||
| return code in this._map; |
There was a problem hiding this comment.
We could avoid using the in operator here by changing this to:
return this._map[code] !== undefined;
There was a problem hiding this comment.
What's wrong with the in operator?
There was a problem hiding this comment.
According to micro-benchmarks, in is two orders of magnitude slower in modern browsers; see e.g. http://jsperf.com/in-vs-not-undefined.
This makes it easier for the representation to be improved.
This change avoids the element stringification caused by for..in for the vast majority of CMaps. When loading the PDF from issue mozilla#4580, this change reduces peak RSS from ~650 to ~600 MiB, and improves overall speed by ~20%, from 902 ms to 713 ms. Other CMap-heavy documents will also see improvements.
|
I changed the |
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.22.172.223:8877/8511673d5ea5388/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/809612b2f34f871/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/8511673d5ea5388/output.txt Total script time: 21.09 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/809612b2f34f871/output.txt Total script time: 23.14 mins
|
|
/botio-linux lint |
From: Bot.io (Linux)ReceivedCommand cmd_lint from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/4257397abdcdd7d/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/4257397abdcdd7d/output.txt Total script time: 0.78 mins
|
|
The travis failure appears to be not my fault. |
BTW, microbenchmarks are hard to write well. For example, the microbenchmark you mentioned measures normal (named) property accesses, but what's relevant for this code is array element accesses, and I've been told that Firefox is much faster on the latter case. See https://bugzilla.mozilla.org/show_bug.cgi?id=1046792 for more details. |
Avoid expensive for..in loops involving CMaps
|
Thank you for the patch |
This is much the same as #4804, which was abandoned.