Closed
Bug 850534
Opened 12 years ago
Closed 12 years ago
Yarr performance improvements
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: h4writer, Assigned: h4writer)
References
Details
Attachments
(2 files)
5.33 KB,
patch
|
dmandelin
:
review+
|
Details | Diff | Splinter Review |
2.65 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
This is mainly by looking at the diff between our yarr folder and webkit's. It consist of reserving the memory of vector before doing append. A second trick is using "swap" to get the contents of the vector, instead of copying it. (This is possible as the original vector isn't used anymore. We already did a similar trick for one of the two vectors, but instead used copy and clear).
This is good for 6% on v8-regexp.
Assignee | ||
Comment 1•12 years ago
|
||
Annotate pointed into your direction, Dave. Feel free to nominate somebody else.
Assignee: general → hv1989
Attachment #724270 -
Flags: review?(dmandelin)
Updated•12 years ago
|
Attachment #724270 -
Flags: review?(dmandelin) → review+
Assignee | ||
Comment 2•12 years ago
|
||
Assignee | ||
Comment 3•12 years ago
|
||
I'll investigate tomorrow, but awfy somehow doesn't show this 6% improvement ?!
Assignee | ||
Comment 4•12 years ago
|
||
When measuring on my computer I took an average. And looks like before this commit v8-regexp was really jumping up and down here. With this patch it is dead stable. Somehow awfy didn't had this jumping and therefore also not the performance increase, booh, but it will definitely decrease the memory usage, yeay.
Assignee | ||
Comment 5•12 years ago
|
||
Looks like octane-regexp did get this improvement, a 4% one :D
Comment 6•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Assignee | ||
Comment 7•12 years ago
|
||
I think this caused the crashes mentioned in bug 825243. Backout just to be sure.
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e1241c6d2ce
Depends on: 825243
Comment 8•12 years ago
|
||
Backout:
https://hg.mozilla.org/mozilla-central/rev/3e1241c6d2ce
However, we still hit the same crash after backing out, so I think you're OK to re-land this.
Updated•12 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 9•12 years ago
|
||
Waldo suggested to use this for when we are failing allocating in Vector...
Attachment #725359 -
Flags: review?
Assignee | ||
Updated•12 years ago
|
Attachment #725359 -
Flags: review? → review?(dmandelin)
Assignee | ||
Updated•12 years ago
|
Attachment #725359 -
Flags: review?(dmandelin) → review?(dvander)
Assignee | ||
Comment 10•12 years ago
|
||
Because this wasn't the reason for the crashes, I pushed again.
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7ea5daa2dc9
Whiteboard: [leave open]
Comment on attachment 725359 [details] [diff] [review]
Use MOZ_CRASH when failing to allocate
Review of attachment 725359 [details] [diff] [review]:
-----------------------------------------------------------------
I guess this will be better than what's there, but I can guarantee we'll see OOM crash reports. Is there any way we can propagate OOM?
Attachment #725359 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 12•12 years ago
|
||
Looks like we indeed see OOM crashes there. Original bug to propagate OOM is bug 574459 open since FF4.0 :(
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c41bf87b4e5
Whiteboard: [leave open]
Comment 13•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a7ea5daa2dc9
https://hg.mozilla.org/mozilla-central/rev/2c41bf87b4e5
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•