476745 - We need to ensure DEBUGGER builds are tested with/without -Dnodebugger

Status: VERIFIED FIXED

(Tamarin Graveyard :: Build Config, defect, P3)

Description

[Steven Johnson]

There are now at least 3 distinct run-modes for avmshell: 
(1) DEBUGGER not defined (aka a release build)
(2) DEBUGGER defined and debugger enabled (aka release-debugger build)
(3) DEBUGGER defined and debugger not enabled (aka release-debugger with -Dnodebugger)

Option (3) is a recent addition (as a result of work on https://bugzilla.mozilla.org/show_bug.cgi?id=467382); running in this mode has considerably less memory and performance overhead vs. Option (1). However, we need to ensure that our buildbots run acceptance/performance tests on all 3 (they are currently only running (1) and (2) I suspect) for obvious reasons.

Note 1: it should eventually be the case that (1) and (3) have nearly-identical performance and memory profiles; currently performance is virtually the same but there is still memory-reduction work to be done.

Note 2: perhaps we should make -Dnodebugger the default? i.e., only have avmshell instantiate a debugger if -d is passed on the command line. (although, since -d causes a breakpoint upon loading, we probably would need another optional, eg "-Dforcedebugger", for running acceptance/performance tests...)

Comment 1

[Lars T Hansen]

If (1) and (3) have the same performance and will have relatively the same memory use, should we even support (1)?  The testing burden grows very quickly with the number of possible configurations, so reducing the number of configurations can have a significant impact on actual quality of the product.

Comment 2

[Edwin Smith]

the traditional argument for non-DEBUGGER configs has been code size.  whatever we do, we need a good measurement for what DEBUGGER really costs (code, memory, etc); the smaller that delta is, the harder it is to justify against the cost of 2*N configs (3*N, actually, given this bug's premise).

A big contributer to code size is the parameters to exception messages; each site where an exception could be thrown gets smaller, and there are ~1000+ such sites.  with some refactoring this can be shrunk by a lot.

A secondary size contributor is the size of the debugger/profiler "rigging" and the size of exception message strings.  IIRC in TT, all these added up to somethin on the order of 25-50K in the thumb config.

Comment 3

[Steven Johnson]

I agree 100% with Lars wrt the testing burden, but it's likely that small-device builds will want to eliminate all unnecessary code with extreme prejudice -- unless we can get the code-size overhead of DEBUGGER mode to something very small, it's unlikely we can nuke it.

Comment 4

[Steven Johnson]

In any event, the immediate-action item here is to ensure that DEBUGGER-without-debugger builds are part of the buildbot process.

Comment 5

[Lars T Hansen]

(In reply to comment #3)
> I agree 100% with Lars wrt the testing burden, but it's likely that
> small-device builds will want to eliminate all unnecessary code with extreme
> prejudice -- unless we can get the code-size overhead of DEBUGGER mode to
> something very small, it's unlikely we can nuke it.

Right.

So let's aim for making the delta between DEBUGGER and non-DEBUGGER mode as small as possible - ideally, the error strings.  At that point we can probably stop testing one of the configs on a per-checkin basis.

Comment 6

[Brent Baker]

Running of acceptance suite in a DEBUGGER shell with -Dnodebugger has been added into the build system via changeset 2370:97fb4878bedc for both RELEASE and DEBUG builds of the shell.

http://hg.mozilla.org/tamarin-redux/rev/97fb4878bedc