1181869 - Make Bindings use the normal Rooted infrastructure

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Terrence Cole [:terrence]]

It's currently using the wonky InternalHandle thing because we wanted to avoid a bunch of extraneous stack copies. What I didn't realize at the time is that the actual Binding* is external, so the struct itself is a svelt 40 bytes. Copying these to and from the stack is not ruinous at all, allows us to use normal Rooted techniques, and will allow us to remove InternalHandle.

Comment 1

[Terrence Cole [:terrence]]

Attachment: cleanup_bindings_rooting-v0.diff

Comment 2

[Shu-yu Guo [:shu]]

Comment on attachment 8631635 [details] [diff] [review]
cleanup_bindings_rooting-v0.diff

Review of attachment 8631635 [details] [diff] [review]:
-----------------------------------------------------------------

This is a nice cleanup.

My only dislike is that almost always we pull the Bindings out only to copy it back into JSScript, largely due to this temporary vs script storage nonsense. It'd be nice to clean that up so we have some stack-allocated Bindings that get copied to script storage wholesale instead of having its "storage switched".

If we don't switch that soon, it would be nice to have some RAII thing that combines the rooting + copying back into JSScript.

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d588ff0a68d9

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d588ff0a68d9

Comment 5

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b3d242f2cf2c

Comment 6

[Terrence Cole [:terrence]]

No change after backout. The regression appears to be noise.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/de6a79687e70

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/de6a79687e70