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

NEW
Unassigned

Status

()

Core
JavaScript Engine
--
major
4 years ago
4 years ago

People

(Reporter: Waldo, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

4 years ago
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.
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.
(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

4 years ago
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.
(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...
(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.
You need to log in before you can comment on or make changes to this bug.