527803 - avoiding unnecessary js_AddRoot

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Igor Bukanov]

Attachment: v1

In couple of places in jsdbgapi.cpp the code uses js_AddRoot when that can be avoided. The first place is JS_SetTrap where js_AddRoot roots trap's closure. That is not necessary since the engine can simply enumerate all the traps during the GC and mark all trap closures is theyr are GC things. The second place is JS_GetPropertyDesc. There js_AddRoot can be replaced with JSAutoTempValueRooter.

The patch below removes these unnecessary calls and adds an api test to check the trap closure rooting.

Comment 1

[Brendan Eich [:brendan]]

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

>@@ -499,11 +499,17 @@ js_ResumeVtune(JSContext *cx, JSObject *
> extern JS_FRIEND_API(JSBool)
> js_InitEthogram(JSContext *cx, JSObject *obj,
>                 uintN argc, jsval *argv, jsval *rval);
> extern JS_FRIEND_API(JSBool)
> js_ShutdownEthogram(JSContext *cx, JSObject *obj,
>                     uintN argc, jsval *argv, jsval *rval);
> #endif /* MOZ_TRACEVIS */
> 
>+/*
>+ * Internal engine API's.
>+ */
>+extern void
>+js_MarkTraps(JSTracer *trc);

We have other js_* prototypes, even unifdef'ed in many cases, but I wonder if this one couldn't just go in jsgc.cpp instead of here. Not a big deal either way.

Thanks for cleaning up this old code!

/be

Comment 2

[Igor Bukanov]

Attachment: v2

The new version moves the declaration of js_MarkTraps into jsgc.h

Comment 3

[Igor Bukanov]

https://hg.mozilla.org/tracemonkey/rev/468c1592469f

Comment 4

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/468c1592469f