Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

bail early on closestExpression#3210

Merged
codehag merged 3 commits into
firefox-devtools:masterfrom
jasonLaster:speedup-parse
Jun 23, 2017
Merged

bail early on closestExpression#3210
codehag merged 3 commits into
firefox-devtools:masterfrom
jasonLaster:speedup-parse

Conversation

@jasonLaster
Copy link
Copy Markdown
Contributor

@jasonLaster jasonLaster commented Jun 23, 2017 •
edited
Loading

Associated Issue: #3209

This offers much needed perf wins when debugging large 10K+ files (a.k.a. the debugger.js file). Previously the perf times were a couple seconds for finding the closest expression and out of scope lines and now they're a couple hundred milliseconds or less...

It's amazing how much it pays to be lazy.

I got these rough stats from adding logging to our worker utils

 37         var t0 = performance.now();
 38         this.worker.postMessage({ id, method, args });
 39
 40         const listener = ({ data: result }) => {
 41           if (result.id !== id) {
 42             return;
 43           }
 44
 45           this.worker.removeEventListener("message", listener);
 46           var t1 = performance.now();
 47           console.log(`${method} took:` + (t1 - t0) + " milliseconds.");
 48

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 23, 2017 •
edited
Loading

Codecov Report

Merging #3210 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3210      +/-   ##
==========================================
- Coverage   47.94%   47.93%   -0.02%     
==========================================
  Files          98       98              
  Lines        4063     4062       -1     
  Branches      838      836       -2     
==========================================
- Hits         1948     1947       -1     
  Misses       2115     2115
Impacted Files Coverage Δ
src/utils/parser/getSymbols.js 100% <ø> (ø) ⬆️
src/utils/parser/getOutOfScopeLocations.js 84% <100%> (-1.72%) ⬇️
src/utils/parser/utils/closest.js 92.5% <100%> (+0.39%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a611465...7dc242c. Read the comment docs.

codehag
codehag approved these changes Jun 23, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@codehag codehag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great, minor nit

Comment thread src/utils/parser/getOutOfScopeLocations.js Outdated
let location = Object.assign({}, get("node.loc", path));

function getLocation(func) {
let location = Object.assign({}, func.location);
Copy link
Copy Markdown
Contributor

@codehag codehag Jun 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this should be
const location = { ...func.location };

@codehag
Copy link
Copy Markdown
Contributor

codehag commented Jun 23, 2017

i will fix up the comment and the tests

@codehag codehag merged commit 120b400 into firefox-devtools:master Jun 23, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@codehag codehag codehag approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jasonLaster @codehag