393184 - Avoid unnecessary zeroing of JSGCThing

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Igor Bukanov]

The current code of js_NewGCThing in jsgc.c contains:

    /*
     * Clear thing before unlocking in case a GC run is about to scan it,
     * finding it via newborn[].
     */
    thing->next = NULL;
    thing->flagp = NULL;

Such zeroing here is unnecessary since all users of js_NewGCThing initialize the thing before doing any operation that can trigger GC. Moreover, the code does not work as a defense again bugs in the initialization code. For that it would be necessary to call memset(thing, 0, nbytes) to clear the whole thing, not just  first sizeof(JSGCThing) its bytes.

Thus I suggest to remove this zeroing.

Comment 1

[Igor Bukanov]

Attachment: v1

js_NewObject is the only user of js_NewGCThing that relies on JSGCThing zeroing so a potential GC called from newObjectMap would see null, not junk, as JSObject.map. Thus the patch added explicit obj->map = NULL besides removing the zeroing code in js_NewGCThing.

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 277712 [details] [diff] [review]
v1

>+ * can potentially cause GC. This will ensure that GC tracing never see a junk
>+ * values stored in the partially initialized thing.

s/see a/sees/

r/a=me with that.

/be

Comment 3

[Igor Bukanov]

Attachment: v1b

Patch to check in with nits addressed.

Comment 4

[Igor Bukanov]

I checked in the patch from comment 4 to the trunk:

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%2Fcvsroot&date=explicit&mindate=1187816103&maxdate=1187816280&who=igor%mir2.org