If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

VERIFIED FIXED in flash10.1

Status

Tamarin
Build Config
P3
normal
VERIFIED FIXED
9 years ago
8 years ago

People

(Reporter: Steven Johnson, Assigned: Brent Baker)

Tracking

unspecified
flash10.1
Bug Flags:
in-testsuite +
flashplayer-qrb +
flashplayer-triage +

Details

(Reporter)

Description

9 years ago
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

9 years ago
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

9 years ago
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.
(Reporter)

Comment 3

9 years ago
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.
(Reporter)

Comment 4

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

Comment 5

9 years ago
(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.
(Assignee)

Updated

9 years ago
Flags: in-testsuite?
Flags: flashplayer-triage+
Flags: flashplayer-qrb?
(Assignee)

Updated

9 years ago
Depends on: 467382

Updated

8 years ago
Assignee: nobody → brbaker
Status: NEW → ASSIGNED
Flags: flashplayer-qrb? → flashplayer-qrb+
Priority: -- → P3
Target Milestone: --- → flash10.x

Updated

8 years ago
Component: Virtual Machine → Build Config
QA Contact: vm → build-config
(Assignee)

Comment 6

8 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Updated

8 years ago
Status: RESOLVED → VERIFIED
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.