Last Comment Bug 766004 - Remove LookupCompileTimeConstant()
: Remove LookupCompileTimeConstant()
Status: RESOLVED FIXED
[js:t]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla16
Assigned To: Nicholas Nethercote [:njn]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: LazyBytecode
  Show dependency treegraph
 
Reported: 2012-06-18 18:39 PDT by Nicholas Nethercote [:njn]
Modified: 2012-06-20 02:18 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (8.74 KB, patch)
2012-06-18 18:39 PDT, Nicholas Nethercote [:njn]
bhackett1024: review+
Details | Diff | Splinter Review

Description Nicholas Nethercote [:njn] 2012-06-18 18:39:40 PDT
Created attachment 634266 [details] [diff] [review]
patch

LookupCompileTimeConstant() is a problem for lazy bytecode.  It potentially traverses along bce->parent from the innermost BytecodeEmitter all the way to the top of the BytecodeEmitter chain.  But if we're lazily compiling an inner function we won't have the parent BytecodeEmitter around any more.

It's there just to optimize the handling of |const| variables in switches, e.g.:

  const x = "foo"
  switch (y) {
    case x: ...
    ...
  }

It's an old optimization, pre-dating the CVS-to-Mercurial transition in 2007.  And I don't think |const| is used that much on the web, is it?

bhackett said "it's antique code and if there is any need for what it's doing we can do a more effective job using TI info during mjit compilation".
Comment 1 Luke Wagner [:luke] 2012-06-18 18:43:41 PDT
I also found, in the case of huge generated JS files, that it takes like 10% of total frontend time.
Comment 2 Nicholas Nethercote [:njn] 2012-06-19 20:13:29 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/666e228c06ab
Comment 3 Ed Morley [:emorley] 2012-06-20 02:18:37 PDT
https://hg.mozilla.org/mozilla-central/rev/666e228c06ab

Note You need to log in before you can comment on or make changes to this bug.