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.

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

Proposal: Introduce codemod tools for long-term refactoring works #38609

Open
pd4d10 opened this issue May 9, 2021 · 5 comments
Open

Proposal: Introduce codemod tools for long-term refactoring works #38609

pd4d10 opened this issue May 9, 2021 · 5 comments
Labels
discuss Issues opened for discussions and feedbacks.

Comments

@pd4d10
Copy link
Contributor

pd4d10 commented May 9, 2021

I noticed that there has been a lot of work to refactor the codebase, for example:

I guess codemod tools may help in these cases. Actually this PR #38608 is finished by the codemod script here https://github.com/pd4d10/nodejs-codemod (a rough MVP version, may have some bugs)

Can't say it saves lots of time in this case, because there are not many changes. But it may be useful in the future, especially as the codebase grows.

@marsonya
Copy link
Member

This could be very helpful but the problem is that in some cases, primordials cause significant regressions. Until we have optimisations from V8, we cannot have a blanket implementation of primordials.

We already have some instances where we had to revert the use of primordials to deal with significant regressions.
See #38248 and #38246

@marsonya marsonya added the discuss Issues opened for discussions and feedbacks. label May 10, 2021
@Ayase-252
Copy link
Member

Maybe we can find which primoridials are good to perf. with tools by choosing subset of promoridials to refactor?

@Linkgoron
Copy link
Member

Linkgoron commented May 10, 2021

I'm also -1 on codemods for primordials (I'm ±0 in general, depending on the use-case I guess), as we've seen that they create performance issues and they shouldn't be added without some thought, and at least some benchmarking.

Maybe we can find which primoridials are good to perf. with tools by choosing subset of promoridials to refactor?

I assume that static function primordials are probably relatively safe (MathMax etc.), however others probably less so. IIRC Array methods were especially problematic, as were FunctionApply etc.

Even so, maybe even for static functions, it might be hard to see before-hand how stuff would hurt inlining (e.g. #38246 (comment)) and if there's actually a good general rule for changing stuff.

@bmeck
Copy link
Member

bmeck commented May 10, 2021

We could probably at least do codemods to automate generating out code gen traces like we did with https://bugs.chromium.org/p/v8/issues/detail?id=9702&q=bound%20apply&can=1 to check for regressions rather than doing it manually and then open up any issues upstream

@avivkeller
Copy link
Member

Hey, is this still being worked on? If not, feel-free to self-close the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issues opened for discussions and feedbacks.
Projects
None yet
Development

No branches or pull requests

6 participants