GNOME Bugzilla – Bug 691039
Add a wrapper for JS_DumpHeap
Last modified: 2013-07-19 20:11:47 UTC
This is designed to be used by JS code to help debug GC cycles and other such issues.
Created attachment 232591 [details] [review] configure: Remove CLASS_TRACE gunk This was removed when we added support for js187, and was broken anyway.
Created attachment 232592 [details] [review] configure: Move useless echo lines JS_CFLAGS and JS_LIBS don't exist
Created attachment 232593 [details] [review] jsapi-util: Rewrite argument counting logic We're going to need this for maybe args, so let's clean up the code now.
Created attachment 232594 [details] [review] jsapi-util: Add a maybe prefix flag for gjs_parse_args This allows users to have "maybe" values where the C value is treated as NULL.
Created attachment 232595 [details] [review] system: Add a wrapper for JS_DumpHeap If JS was compiled with debug information, it will have JS_DumpHeap among others, which provide valuable debugging insight.
Review of attachment 232595 [details] [review]: ::: gjs/jsapi-private.cpp @@ +48,3 @@ +#undef ADDED_DEBUG +#endif /* HAVE_JS_DUMPHEAP */ + Why do you need this dance inside jsapi-private.cpp?
Review of attachment 232591 [details] [review]: JS_CLASS_TRACE and JS_MARK_IS_TRACE are still needed on js185.
Review of attachment 232592 [details] [review]: Yes.
Review of attachment 232595 [details] [review]: ::: gjs/jsapi-private.cpp @@ +48,3 @@ +#undef ADDED_DEBUG +#endif /* HAVE_JS_DUMPHEAP */ + Because it fails to compile without it. I'd love to not do the dance, but I'm not sure of a better way to do it.
Review of attachment 232593 [details] [review]: Ok
Review of attachment 232591 [details] [review]: Except we never use HAVE_JS_CLASS_TRACE. And if you look in your log files, the answer is always "no" because it can't find jsapi.h because this is ran before the CFLAGS are around. The commit message is wrong, though.
Review of attachment 232594 [details] [review]: Ok (gettext needs to die, replaced with JS and introspection. I have a patch somewhere...)
Created attachment 232686 [details] [review] configure: Remove HAVE_JS_CLASS_TRACE gunk This is unused and was broken, as it didn't have the correct CFLAGS, leading to a "jsapi.h is not found" error. Fixed up commit message. Re-review?
Attachment 232592 [details] pushed as 1738678 - configure: Move useless echo lines Attachment 232593 [details] pushed as 86b380d - jsapi-util: Rewrite argument counting logic Attachment 232594 [details] pushed as ef4e523 - jsapi-util: Add a maybe prefix flag for gjs_parse_args
Review of attachment 232595 [details] [review]: Two minor things, but feel free to commit just addressing the first. ::: configure.ac @@ +115,3 @@ + [have_js_dumpheap=no]) + +if test "x$have_js_dumpheap" = xyes; then Use AS_IF() here. ::: modules/system.c @@ +113,3 @@ + JSObject *target_obj; + if (!gjs_parse_args(context, "dumpHeap", "?o", argc, argv, "object", &target_obj)) + return JS_FALSE; Seems like it'd be nice to support redirecting to a filename and not just stderr.
This is complicated, and it's not going to work.