Skip to content
This repository has been archived by the owner. It is now read-only.

Use fast-url-parser #8708

Merged
merged 1 commit into from May 8, 2017
Merged

Use fast-url-parser #8708

merged 1 commit into from May 8, 2017

Conversation

@ayumi
Copy link
Contributor

ayumi commented May 5, 2017

Use faster implementation of url.parse().

Related to #7453. With 10K bookmarks, the URL bar first letter latency reduces from 2600 ms to 1700 ms.

Fix #8707

Auditors: @bsclifton @bbondy

Test Plan:

Related benchmark:

~/repos/github.com/petkaantonov/urlparser(master*) % node -v                                                                                          23:39:48
v7.9.0
~/repos/github.com/petkaantonov/urlparser(master*) % node ./benchmark/nodecore.js                                                                     23:39:53
misc/url.js parse(): 67219.592
misc/url.js format(): 50081.595
misc/url.js resolve("../foo/bar?baz=boom"): 49206.484
misc/url.js resolve("foo/bar"): 54228.107
misc/url.js resolve("http://nodejs.org"): 49014.074
misc/url.js resolve("./foo/bar?baz"): 54074.062
~/repos/github.com/petkaantonov/urlparser(master*) % node ./benchmark/urlparser.js                                                                    23:39:58
misc/url.js parse(): 246442.84
misc/url.js format(): 139617.20
misc/url.js resolve("../foo/bar?baz=boom"): 208489.18
misc/url.js resolve("foo/bar"): 216373.50
misc/url.js resolve("http://nodejs.org"): 144010.28
misc/url.js resolve("./foo/bar?baz"): 215226.83
Fix #8707

Test Plan:
Make sure automated tests pass.
@ayumi ayumi force-pushed the feature/fast-url-parser branch from 3b39964 to 7b0f7d5 May 5, 2017
// In fast-url-parser href, port, prependSlash, protocol, and query
// are lazy so access them.
const raw = urlParse(url, ...args)
parsedUrl = {

This comment has been minimized.

Copy link
@bridiver

bridiver May 5, 2017

Collaborator

why copy all the properties here?

This comment has been minimized.

Copy link
@ayumi

ayumi May 5, 2017

Author Contributor

The first 5 properties are lazy / accessor functions, so this is to populate those properties.

This comment has been minimized.

Copy link
@bridiver

bridiver May 8, 2017

Collaborator

ah, I see, so it breaks when it gets cloned in Object.assign

@bridiver bridiver merged commit 8afac1a into master May 8, 2017
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
@bridiver
Copy link
Collaborator

bridiver commented May 8, 2017

++

@bridiver bridiver deleted the feature/fast-url-parser branch May 8, 2017
@ayumi ayumi mentioned this pull request May 9, 2017
@luixxiul luixxiul added this to the 0.15.3 milestone May 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.