Closed Bug 786743 Opened 8 years ago Closed 8 years ago

Need real variable names for debugging self-hosted JavaScript

Categories

(Core :: JavaScript Engine, defect)

Other Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: mozillabugs, Assigned: till)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Since all self-hosted JavaScript code currently is minified, Error messages relating to this code can have only minified variable names. In a larger program, the name "a" can occur in many functions, making it hard to track down which variable was meant.

There should be a way to get the original variable names in debug builds. The js2c.py script seems to hint at a more comprehensive debugging solution, but as a first step just using the original source code rather than a minified form in debug builds would help.
This patch disables minification for debug builds. The enticingly-looking bits about debugging in JS2C.py are unfortunately related to V8's debug capabilities and thus not related to debugging the self-hosted code itself.

One thing to consider is a flag that can be set to disable the hiding of source code and stack frames for self-hosted code. That could only ever be a runtime-settable flag instead of a build flag or something that's always activated for debug builds. Otherwise, the difference in behavior would certainly be too big.
Assignee: general → tschneidereit
Status: NEW → ASSIGNED
Attachment #656884 - Flags: review?(luke)
Comment on attachment 656884 [details] [diff] [review]
Disable JS minification in debug builds

Logically the change makes sense, but I'm completely unfamiliar with this code (and our build system in general...); could you use the original reviewer of this code?
Attachment #656884 - Flags: review?(luke)
Comment on attachment 656884 [details] [diff] [review]
Disable JS minification in debug builds

Right, I didn't think the request through. Sorry for the noise, Luke.
Attachment #656884 - Flags: review?(ted.mielczarek)
I don't know the first thing about this stuff, but I wonder:  how important is the minification for performance?
(In reply to Nicholas Nethercote [:njn] from comment #4)
> I don't know the first thing about this stuff, but I wonder:  how important
> is the minification for performance?

It shouldn't matter for performance at all. It does, however, decrease code size. 

Also, I don't know enough about how the bytecode cloning and source code retaining works to know this, but it might also be important for memory use as all self-hosted functions get cloned into globals using them.
Comment on attachment 656884 [details] [diff] [review]
Disable JS minification in debug builds

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

::: js/src/Makefile.in
@@ +839,5 @@
>    $(srcdir)/builtin/macros.py \
>    $(srcdir)/builtin/js2c.py \
>    $(srcdir)/builtin/embedjs.py
>  
> +selfhosting_embed_flags := $(NULL)

You don't need this, you can += to an unset variable and it will work fine.

::: js/src/builtin/embedjs.py
@@ +36,5 @@
>      return line
>  
>  def main():
> +    files_offset = 0
> +    debug = sys.argv[1] == '-d'

This is fine for now. If this script grows any more options you'll want to switch to using OptionParser.

@@ +38,5 @@
>  def main():
> +    files_offset = 0
> +    debug = sys.argv[1] == '-d'
> +    if debug:
> +        files_offset = 1

Just do sys.argv.pop(1) instead of this fiddling.
Attachment #656884 - Flags: review?(ted.mielczarek) → review+
https://hg.mozilla.org/mozilla-central/rev/2f83e74b38ff
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.