js_RegisterCloseableIterator uses a runtime-wide lock

RESOLVED FIXED in mozilla11

Status

()

Core
JavaScript Engine
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: gal, Assigned: evilpie)

Tracking

Trunk
mozilla11
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
This hits every for-in loop, and is horribly slow. This should be per-thread.

JS_LOCK_GC(rt);
ok = rt->gcIteratorTable.append(obj);
JS_UNLOCK_GC(rt);

Comment 1

7 years ago
iirc this is because workers can migrate from thread to thread. ack!
When that happens, they can move their iterators into an expensive runtime thing and flag them so that we remove from the expensive runtime thing later.

(Are threads really that expensive that we can't have one per worker?  I guess it's a lot of non-pageable memory on some systems.)
I think we can get rid of this table altogether with some work on iteration. Don't panic!

/be

Updated

7 years ago
Depends on: 554866

Comment 4

7 years ago
(In reply to comment #1)
> iirc this is because workers can migrate from thread to thread. ack!

Yes, this is indeed the case. I should have filed a bug to support migration of JSThread to another native thread long time ago. But it is better late than never, bug 554866.
(Assignee)

Comment 5

6 years ago
Okay this function was removed, but not the definition in jsgc.h oO
(Assignee)

Comment 6

6 years ago
Created attachment 576822 [details] [diff] [review]
rm
(Assignee)

Updated

6 years ago
Attachment #576822 - Flags: review?(Ms2ger)
Comment on attachment 576822 [details] [diff] [review]
rm

I bet nobody will mind... rs=me
Attachment #576822 - Flags: review?(Ms2ger) → review+
(Assignee)

Updated

6 years ago
Assignee: general → evilpies
(Assignee)

Comment 8

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/05453377527a
https://hg.mozilla.org/mozilla-central/rev/05453377527a
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.