1055301 - Removing the error message numbers means that adding any error message except at end, or removing a message, requires bumping XDR version

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

Self-hosting code burns in error message numbers right now, for the purpose of calling ThrowError.  That means XDR of self-hosted functions is affected, which means to a good approximation changing js.msg now requires an XDR-bump, where before it only required it if one were changing an error used in self-hosting code (which would much more readily be part of the corresponding patch).  This seems dangerous to me.

I don't know what the solution is here, but the current situation, post-bug 1054322, seems far too error-prone.

Comment 1

[Jan de Mooij [:jandem]]

IMO the XDR version bump has always been kind of error-prone. Pretty much all patches I review that modify the bytecode have a "You should bump the XDR version too" comment.

The asm.js caching uses the build id, I wonder if we could do something similar here.

Comment 2

[Till Schneidereit [:till]]

(In reply to Jan de Mooij [:jandem] from comment #1)
> IMO the XDR version bump has always been kind of error-prone. Pretty much
> all patches I review that modify the bytecode have a "You should bump the
> XDR version too" comment.

Agreed, I probably forgot bumping the XDR version in half of the patches I would've needed to do so.

> 
> The asm.js caching uses the build id, I wonder if we could do something
> similar here.

I like it. The downside would be that Nightly users would get their caches invalidated daily, as opposed to every few days to weeks. I guess that wouldn't be the end of the world, though.

Comment 3

[Luke Wagner [:luke]]

For reference, the buildid can be retrieved via rt->asmJSCacheOps.buildId which we could move to rt->buildIdOp.

One caveat I've seen using the buildid: the buildid only gets bumped when a new firefox executable is built so not all local builds trigger generate a new buildid.  In particular, if you do "make -C js/src && make -C toolkit/library" (as I usually do) that only builds a new libxul, so you don't get a new buildid.  This isn't terrible (there's even a -purgecaches flag that flushes XDR so you can avoid rebuilding the exe); but it can lead to wasted time tracking down fake bugs, especially, e.g., if you have a bug in a stack of patches and you are trying to pop to find the which one.  I guess we could fix this by making a new js-build-id that lived in js/src; if it's not hard, it'd probably be worth it.

Comment 4

[Jan de Mooij [:jandem]]

(In reply to Luke Wagner [:luke] from comment #3)
> I guess we could fix this by making a new js-build-id that lived in js/src; if
> it's not hard, it'd probably be worth it.

As far as I can see, the asm.js build id comes from GRE_BUILDID, which the build system defines based on $objdir/config/buildid This file is written ifndef JS_STANDALONE:

http://mxr.mozilla.org/mozilla-central/source/config/Makefile.in#50

Defining a JS_BUILDID and using that somehow seems doable. One thing is, do we want "make" for the shell to always update the build id and hence always build + link something? That could be annoying...

Comment 5

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #4)
> Defining a JS_BUILDID and using that somehow seems doable. One thing is, do
> we want "make" for the shell to always update the build id and hence always
> build + link something? That could be annoying...

Same issue with the browser build: if SpiderMonkey depends on the build id, we risk rebuilding libxul on incremental builds, not ideal.

Comment 6

[Ted Campbell [:tcampbell]]

(We ended up using BuildId and ran into the exact concerns described 5 years ago.. In addition, we will probably now need to hash some files for a second version number)