1262595 - declare js::ConstNullValue as a const value

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nathan Froyd [:froydnj]]

This change avoids multiple copies of ConstNullValue being generated in
every compilation unit that includes RootingAPI.h.

Comment 1

[(Away)]

ShowGlobals says that ConstNullValue is our most-duplicated symbol at 104 copies, one for each translation unit that includes RootingAPI.h (and presumably much worse in non-unified builds).

Comment 2

[(Away)]

Attachment: Static in Handle's ctor

This patch saves around 800 bytes by bringing us down to a single-digit handful of duplicates, one for each template instantiation of Handle. It seemed like the most bang per buck. If we wanted to drive this all the way down to 1, I guess we could define something in a .cpp, but I didn't think a few more bytes was worth moving the definition far away.

Comment 3

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8835134 [details] [diff] [review]
Static in Handle's ctor

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

Funky. Definitely ok with this. Just to check -- this will get placed in a read-only section, right? I would prefer if writes through one of these Handles would trigger an immediate crash rather than corrupting a shared "null" value.

Comment 4

[(Away)]

That's definitely a fair concern. I've just checked and indeed these live in .rdata and are marked PAGE_READONLY at runtime.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9930b3b01f8f
Reduce duplication of ConstNullValue across compilation units. r=sfink

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9930b3b01f8f