558211 - Eliminate JS_SetObjectHook

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

I'm not sure we have any debuggers actually using this. It's a little hard to tell, because JSD enables the object hook by default, but disables it if the client asks for JSD_DISABLE_OBJECT_TRACE, aka jsdIDebuggerService.DISABLE_OBJECT_TRACE.

ISTR Firebug uses that flag.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Also worth checking what venkman does.

Comment 2

[Jason Orendorff [:jorendorff]]

Firebug: http://www.google.com/codesearch/p?hl=en#FXMmmf0ln7k/components/firebug-service.js&q=DISABLE_OBJECT_TRACE%20package:http://fbug\.googlecode\.com&sa=N&cd=1&ct=rc&l=700

Venkman: http://mxr.mozilla.org/mozilla/source/extensions/venkman/resources/content/venkman-debugger.js#132

I don't think it's used.

Comment 3

[Jason Orendorff [:jorendorff]]

Attachment: part 1, remove jsd_obj.c - v1

(note: I put all the IDL changes in part 2, coming soon)

Comment 4

[Jason Orendorff [:jorendorff]]

Attachment: part 2, remove jsdIObject from IDL and update uuids - v1

Comment 5

[Jason Orendorff [:jorendorff]]

Attachment: part 3, remove JS_SetObjectHook from the js engine - v1

Comment 6

[Andreas Gal :gal]

Comment on attachment 438882 [details] [diff] [review]
part 3, remove JS_SetObjectHook from the js engine - v1

Good bye new object hook. You won't be missed around here.

Comment 7

[Jason Orendorff [:jorendorff]]

It looks like Venkman is using JS_SetObjectHook. This function is getting metadata about an object that's gathered via the hook:

http://mxr.mozilla.org/mozilla/source/extensions/venkman/resources/content/venkman-commands.js#918

You can look at an object and see where it was allocated -- useful stuff. Off by default though.

There's no measurable performance improvement (on V8 or SunSpider) deleting the object hook, I think because neither JIT code nor anything called from JIT code even tests to see that the hook is set (it is guaranteed not to be set if we're on trace). It might be harmless to just leave it.