660441 - removal of cx parameter from IsAboutToBeFinalized

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

IsAboutToBeFinalized takes cx to access the runtime and get the associated gcCurrentCompartment. Howerever, the runtime can be accessed from the cpmpartment for the thing making the cx parameter unnecessary.

Comment 1

[Gregor Wagner [:gwagner]]

Do you also want to change the JS_IsAboutToBeFinalized function? Whats the procedure to change an API function?

Comment 2

[Igor Bukanov]

Attachment: v1

Here is the patch that removes the unneeded cx parameter.

Comment 3

[Igor Bukanov]

(In reply to comment #1)
> Do you also want to change the JS_IsAboutToBeFinalized function?

Thanks for remainding about it! I will update the patch to always call IsAboutToBeFinalized from xpconnect. After that we would not have JS_IsAboutToBeFinalized calls in the browser and we can wait with any changes to JS_IsAboutToBeFinalized until we have a generational GC or something like that that disrupts the GC logic significantly. 

> Whats the
> procedure to change an API function?

Some discussion on internal and public mail lists is in due.

Comment 4

[Igor Bukanov]

Attachment: v2

The patch removes the cx from all internal and public API - per resent discussion on the mailing list this is OK and desirable.

Comment 5

[Igor Bukanov]

Attachment: v3

Here is the right patch!

Comment 6

[Gregor Wagner [:gwagner]]

Comment on attachment 595186 [details] [diff] [review]
v3


>-{
>-    JS_ASSERT(thing);
>-    JS_ASSERT(!cx->runtime->gcIncrementalTracer);
>-    return IsAboutToBeFinalized(cx, (gc::Cell *)thing);
>+JS_IsAboutToBeFinalized(void *thing)
>+{
>+    gc::Cell * t = static_cast<gc::Cell *>(thing);

Space around *?

nice!

Comment 7

[Igor Bukanov]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d6f1d1fbe13a

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d6f1d1fbe13a