Created attachment 411547 [details] [diff] [review] 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.
Attachment #411547 - Flags: review?(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
Attachment #411547 - Flags: review?(brendan) → review+
Created attachment 411944 [details] [diff] [review] v2 The new version moves the declaration of js_MarkTraps into jsgc.h
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.