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

Prettier much slower when using this plugin #114

Open
DaniloNovakovic opened this issue Nov 15, 2023 · 11 comments
Open

Prettier much slower when using this plugin #114

DaniloNovakovic opened this issue Nov 15, 2023 · 11 comments

Comments

@DaniloNovakovic
Copy link

DaniloNovakovic commented Nov 15, 2023

Hi guys, I really like this plugin, however what I have noticed that it slows down prettier by quite a lot.

For example, here is with this plugin:
image

and without this plugin (when I remove it from plugins in prettierrc)
image

As you can see it essentially adds extra 100ms for each file, which has huge impact since we have big monorepo on our project, so this performance issue is very noticable.

Note: I also tried adding "organizeImportsSkipDestructiveCodeActions": true, but it didn't have any impact on performance.

Additional info:
"prettier": "^3.0.3",
"prettier-plugin-organize-imports": "^3.2.3",
OS: Windows 10

Files are written in typescript + react + emotion.

@simonhaenisch
Copy link
Owner

Hey thanks for reporting! Might be a performance regression (already had two of those iirc, might be time for some performance tests 🙈).

module.exports.findTsconfig = memoize((path) => ts.findConfigFile(path, ts.sys.fileExists));

This function from typescript being very slow was the problem last time so I added memoization.

If neither you nor someone else would be able to drill down on this, it'll probably take me some time to get to this, maybe next week.

@simonhaenisch
Copy link
Owner

simonhaenisch commented Nov 17, 2023

Hm ok by just looking at the code a bit, i think I might have figured it out already. findTsConfig gets the path of the file being formatted, which obviously is different for every execution (unless you format the same file twice), so the memoization never really does anything (when I introduced the memoization, it did fix the performance problem though 🤔).

update 7/2/24: looked into this again, and the memoization is done twice, once for const tsconfig = findTsConfig(path) and once for getCompilerOptions(tsconfig)... the latter was actually the one with the performance problem, that's why the performance regression was fixed. the memoization of findTsConfig is actually useless i think since it'll never run the same path twice within the same process (unless i'm missing sth).

@XantreDev
Copy link

XantreDev commented Feb 17, 2024

@simonhaenisch Is not it better to find all tsconfig paths and just use search inside of the array and use most similar path. How much performace cost has findConfigFile?
Btw, findConfigFile in typescript is not optimal at all if used for more than couple files

@kentcdodds
Copy link

I just tested this myself and didn't notice any significant difference with the plugin enabled/disabled.

@simonhaenisch
Copy link
Owner

@DaniloNovakovic just looking through the issues again and found this. I still haven't had these kinds of performance issues, but just wondering is there a sample file you could share where you have the issue?

@tmannherz
Copy link

@simonhaenisch We're seeing a dramatic slow-down (10x) since upgrading to v4. Here are some stats:

Before upgrading (v3.2.4)

npx prettier --write "./*.{js,mjs}" "resources/**/*.{js,jsx,vue,css,pcss,scss}"

babel.config.js 871ms
cypress.config.mjs 40ms
postcss.config.mjs 14ms
tailwind.config.js 26ms
vite.config.mjs 29ms
resources/css/app.pcss 51ms
resources/css/components/vue-datepicker.css 7ms
resources/css/event-calendar.pcss 3ms
resources/js/app.js 48ms
resources/js/components/AlertBox.vue 790ms
resources/js/components/BasePagination.vue 93ms
resources/js/components/BasicModal.vue 69ms
resources/js/components/Cronofy/AvailabilityRules.vue 44ms
resources/js/components/Cronofy/CalendarSync.vue 38ms
resources/js/components/Event/ActionsMenu.vue 81ms
resources/js/components/Event/Checklist.vue 35ms
resources/js/components/Event/InfoBox.vue 62ms

After upgrading (v4.1.0)

npx prettier --write "./*.{js,mjs}" "resources/**/*.{js,jsx,vue,css,pcss,scss}"

babel.config.js 1009ms
cypress.config.mjs 50ms
postcss.config.mjs 12ms
tailwind.config.js 23ms
vite.config.mjs 27ms
resources/css/app.pcss 42ms
resources/css/components/vue-datepicker.css 5ms
resources/css/event-calendar.pcss 3ms
resources/js/app.js 60ms
resources/js/components/AlertBox.vue 1402ms
resources/js/components/BasePagination.vue 1263ms
resources/js/components/BasicModal.vue 1168ms
resources/js/components/Cronofy/AvailabilityRules.vue 1170ms
resources/js/components/Cronofy/CalendarSync.vue 1226ms
resources/js/components/Event/ActionsMenu.vue 1255ms
resources/js/components/Event/Checklist.vue 1155ms
resources/js/components/Event/InfoBox.vue 1273ms

With organize imports plugin disabled

npx prettier --write "./*.{js,mjs}" "resources/**/*.{js,jsx,vue,css,pcss,scss}"

babel.config.js 453ms
cypress.config.mjs 11ms
postcss.config.mjs 3ms
tailwind.config.js 7ms
vite.config.mjs 7ms
resources/css/app.pcss 41ms
resources/css/components/vue-datepicker.css 5ms
resources/css/event-calendar.pcss 3ms
resources/js/app.js 10ms
resources/js/components/AlertBox.vue 66ms
resources/js/components/BasePagination.vue 41ms
resources/js/components/BasicModal.vue 20ms
resources/js/components/Cronofy/AvailabilityRules.vue 15ms
resources/js/components/Cronofy/CalendarSync.vue 14ms
resources/js/components/Event/ActionsMenu.vue 35ms
resources/js/components/Event/Checklist.vue 11ms
resources/js/components/Event/InfoBox.vue 27ms

@simonhaenisch
Copy link
Owner

@tmannherz thanks for sharing, that repo doesn't happen to be open-source, does it? problem is that at the moment i don't have anything to reproduce this with, or at least the impact is much less significant. (I can still try to figure out which functions are slow)

@tmannherz
Copy link

@tmannherz thanks for sharing, that repo doesn't happen to be open-source, does it? problem is that at the moment i don't have anything to reproduce this with, or at least the impact is much less significant. (I can still try to figure out which functions are slow)

It's not, but if I can reproduce outside of our repo, I'll post back here with more info.

@simonhaenisch
Copy link
Owner

Would you be able to share one slow file maybe, e.g. the babel.config.js (feel free to strip anything sensitive)?

@tmannherz
Copy link

babel.config.js

module.exports = {
    presets: [
        // Turn off '@vue/cli-plugin-babel/preset'
        // '@vue/cli-plugin-babel/preset',
        // veaury babel preset
        '@babel/preset-react',
        'veaury/babel/ReactPreset',
    ],
};

@simonhaenisch
Copy link
Owner

Hm tried reproducing with that file, and it's about 25 ms without and 40 ms with the plugin, with either version 3 or 4 🤷🏻‍♂️ nothing like the 1 second your seeing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants