Closed Bug 883439 Opened 6 years ago Closed 6 years ago

Lazily parse scripts defined within catch blocks

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: bhackett, Unassigned)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
Right now lazy parsing is disabled if a function is found within a catch block, as handling for lazy functions with non-function/global scopes is incomplete.  Unfortunately this affects jQuery, and the octane-code-load benchmark.  I didn't notice this before as the handling for such functions was broken and only fixed late in the game for bug 678037.

The attached patch handles lazy functions defined within catch blocks.  Let blocks should in principle also work, but still disable lazy parsing; let blocks aren't used widely on the web yet and I'd like to wait and improve free variable handling vis a vis lazy functions and block scopes when dealing with them.

On my machine this gets octane-code-load's score up to where it should be, 50-55% better than pre-678037.
Attachment #762991 - Flags: review?(luke)
> On my machine this gets octane-code-load's score up to where it should be,
> 50-55% better than pre-678037

nice!

IIUC, you are not just enabling lazy parsing for catch blocks, but for all let blocks, yes?  In that case, can you handle please handle the StaticScopeIter::BLOCK case in TryConvertFreeName so that all accesses to enclosing let vars don't get deoptimized to be JSOP_NAMEs.
(In reply to Luke Wagner [:luke] from comment #1)
> IIUC, you are not just enabling lazy parsing for catch blocks, but for all
> let blocks, yes?

No, lazy parsing is still disabled for let blocks.  Pretty soon I'd like to fix this, but right now I'm mostly interested in tackling awfy issues and would like to keep the scope of these patches limited if possible.
Comment on attachment 762991 [details] [diff] [review]
patch

Ah, I can see the disableSyntaxParser() calls now before calls to letBlock().  Yes, I'm fine not adding lazy parsing of let, I was just worried the patch was deoptimizing let.

Patch looks good then.
Attachment #762991 - Flags: review?(luke) → review+
This gives another 15.6% improvement on octane-codeloaded. Nice!
https://hg.mozilla.org/mozilla-central/rev/d2e7bdb863f7
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Depends on: 883805
You need to log in before you can comment on or make changes to this bug.