Workers: Share strings across thread boundary

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
10 years ago
a month ago

People

(Reporter: bent.mozilla, Assigned: bent.mozilla)

Tracking

({intermittent-failure})

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(status1.9.2 .2-fixed)

Details

(Whiteboard: [evang-wanted-3.6] )

Attachments

(2 attachments)

Posted patch Patch, v1Splinter Review
We're copying now, and it's unfortunate. We should share while we can.
Attachment #414680 - Flags: superreview?(jst)
Attachment #414680 - Flags: review?(jst)
jst, review ping!
Attachment #414680 - Flags: superreview?(jst)
Attachment #414680 - Flags: superreview+
Attachment #414680 - Flags: review?(jst)
Attachment #414680 - Flags: review+
Comment on attachment 414680 [details] [diff] [review]
Patch, v1

Duh, sorry for the way too long delay :(

r=jst
http://hg.mozilla.org/mozilla-central/rev/bdb83d642185
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
How safe would this be for 3.6.x?  Could be a big perf improvement for a lot of people writing JS stuff out there.
(In reply to comment #4)
> How safe would this be for 3.6.x?  Could be a big perf improvement for a lot of
> people writing JS stuff out there.

I can't think of any reason this would be risky, so after it bakes for a bit I'd be all for backporting. The code hasn't really changed much since 3.5.
Comment on attachment 414680 [details] [diff] [review]
Patch, v1

I'd like to take this on branch since it avoids copies of data passed by workers. Pretty much every single test for workers exercises this code so we'd have known pretty fast if this change had broken anything.
Attachment #414680 - Flags: approval1.9.2.2?
Whiteboard: [evang-wanted-3.6]
Comment on attachment 414680 [details] [diff] [review]
Patch, v1

a1922=beltzner, man, I get nervous taking patches this big on the branch
Attachment #414680 - Flags: approval1.9.2.2? → approval1.9.2.2+
(In reply to comment #8)
> http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ae89af4f7cd0

This appears to have caused the persistent leak on Windows mochitests; see mozilla-1.9.2 tinderboxen:

TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1272 bytes during test execution (threshold set at 484 bytes)
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 159 instances of nsStringBuffer with size 8 bytes each (1272 bytes total)

A similar persistent leak shows up on trunk builds on tryserver, though it doesn't get reported on the mozilla-central tinderboxes for some reason. I did a tryserver build with this patch reverted, and confirmed that the leak disappeared.
Posted patch Leax fixSplinter Review
Yeah, I have no idea why this doesn't bite m-c. Some sort of shutdown ordering thing? Maybe we GC more on m-c than on branch? Who knows.
Duplicate of this bug: 547896
Blocks: 438871
Whiteboard: [evang-wanted-3.6] → [evang-wanted-3.6] [orange]
Whiteboard: [evang-wanted-3.6] [orange] → [evang-wanted-3.6]
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.