Closed Bug 786743 Opened 8 years ago Closed 8 years ago
Need real variable names for debugging self-hosted Java
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
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?
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 == '-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 == '-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/integration/mozilla-inbound/rev/2f83e74b38ff Thanks for the review!
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.