Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

src: fix performance regression in node_file.cc #31340

Closed
bnoordhuis opened this issue Jan 13, 2020 · 3 comments
Closed

src: fix performance regression in node_file.cc #31340

bnoordhuis opened this issue Jan 13, 2020 · 3 comments
Labels
lib / src Issues and PRs related to general changes in the lib or src directory. performance Issues and PRs related to the performance of Node.js.

Comments

@bnoordhuis
Copy link
Member

Commits dcb6929, 4396beb and 8a96d05 turned the O(n) scan in InternalModuleReadJSON() into an O(4n) scan. Fix the performance regression by turning that into a linear scan again.

cc @guybedford @jkrems
@bnoordhuis bnoordhuis added lib / src Issues and PRs related to general changes in the lib or src directory. performance Issues and PRs related to the performance of Node.js. labels Jan 13, 2020
@bnoordhuis bnoordhuis mentioned this issue Jan 13, 2020
Merged
2 tasks
@jkrems
Copy link
Contributor

jkrems commented Jan 13, 2020

Do we know that it's a performance issue or is this more of a nice-to-have? I assume it's non-trivial to add because I don't see a pre-existing implementation of strpos with multiple needles in node?

@ryanmurakami

This comment has been minimized.

Sign in to view
@bnoordhuis bnoordhuis changed the title src: make InternalModuleReadJSON great again src: make InternalModuleReadJSON linear again Jan 14, 2020
@ljharb ljharb changed the title src: make InternalModuleReadJSON linear again src: fix performance regression in node_file.cc Jan 15, 2020
@ljharb
Copy link
Member

ljharb commented Jan 15, 2020

Landed in #31343

@ljharb ljharb closed this as completed Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
lib / src Issues and PRs related to general changes in the lib or src directory. performance Issues and PRs related to the performance of Node.js.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@ljharb @bnoordhuis @jkrems @ryanmurakami