554691 - js_RegisterCloseableIterator uses a runtime-wide lock

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andreas Gal :gal]

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

[Robert Sayre]

iirc this is because workers can migrate from thread to thread. ack!

Comment 2

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

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.)

Comment 3

[Brendan Eich [:brendan]]

I think we can get rid of this table altogether with some work on iteration. Don't panic!

/be

Comment 4

[Igor Bukanov]

(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.

Comment 5

[Tom S [:evilpie]]

Okay this function was removed, but not the definition in jsgc.h oO

Comment 6

[Tom S [:evilpie]]

Attachment: rm

Comment 7

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 576822 [details] [diff] [review]
rm

I bet nobody will mind... rs=me

Comment 8

[Tom S [:evilpie]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/05453377527a

Comment 9

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/05453377527a