776000 - Backout Bug 724810 or add some similar API to pass closure data for external strings

Status: NEW

(Core :: JavaScript Engine, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

https://bugzilla.mozilla.org/show_bug.cgi?id=724810#c7

Comment 1

[Luke Wagner [:luke]]

Oops, I just replied in the original bug, copying here:

It looks like noone was using the closure word when the bug 724810 was filed so Igor just removed it.  However, there is still space in the JSString header to store a closure word (String.d.s.u3) so if you think this is an important use case we can just put back JS_NewExternalStringWithClosure and add the closure parameter to the callback).

Comment 2

[Eddy Bruel [:ejpbruel]]

I'm taking this bug; I should be able to take a look at this next week.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

I believe we could speed up certain DOM APIs 20-30% if we could share nsIAtoms' data to JS.
And for that closure data would be really useful (one could call Release on the atom when finalizing)

Comment 4

[Luke Wagner [:luke]]

Sounds like an important use case :)

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

I could be wrong with the numbers, but I'd like to at least try the approach :)

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Boris, this was the bug I was thinking last week.
Not impossible to add several external string types, but just a bit too hard in certain cases.

If we add dom::String, it might be nice to be able to store pointer to the dom::String object 
in the closure. (I'm imagining approach where dom::String would hold nsStringBuffer and non-thread-safe refcnt)

Comment 7

[Boris Zbarsky [:bzbarsky]]

Hmm.  Maybe.  Would need to think about it.

I'd really prefer us to just fix our string thread stuff.  :(

Comment 8

[Luke Wagner [:luke]]

I just noticed that JSStringFinalizer isn't a function pointer, but a struct containing a function pointer.  That means that an external-string user could embed the struct in a bigger struct and reach whatever closure data at a fixed offset from the JSStringFinalizer*.