Benchmarking against TSLint #55
Labels
documentation
Documentation ("docs") that needs adding/updating
package: eslint-plugin
Issues related to @typescript-eslint/eslint-plugin
Comments
|
Certainly an interesting idea. I'd be curious to see what the overall performance difference between the generic (eslint) and the specialised (tslint) is. We have now got our "roadmap" which lists what tslint has that we don't, and what we recommend for missing features. |
|
@bradzacher I have a very old project (https://gitlab.com/corbinu/eslint-typescript-smoketest) I wrote when the parser was first being written to run it on major typescript projects to find syntax where it errored out completely. I think I could adapt it to benchmarking pretty easily. Either way I do think these projects would be good for benchmarking ( if it goes well maybe we can even try to convince a few to switch ;) )
|
|
Both @typescript-eslint and TSLint use the TypeScript compiler to parse the source code, but then @typescript-eslint has to transform the AST in order for ESLint to consume it. TSLint works based on the TypeScript AST. As a linter ESLint may be faster than TSLint, but there are some unavoidable extra steps in the architecture right now, so on balance I think @typescript-eslint will either be at parity or a bit slower. I could well be wrong (that's why I agree benchmarking is good :) ) HOWEVER, raw speed of linting should be way down the list of priorities. The focus should be on quality, actionable feedback, flexible configuration, auto-fixing, community resources etc. I am not saying we should not benchmark - there will almost certainly be areas we can improve the speed of certain operations on our side - but I want to make sure people understand the moving pieces so that nobody has any unreasonable expectations about the potential results. |
|
i did some e2e tests on big projects, if tsconfig.json has paths specified correctly i see ~small performance impact. i also tried to set it up inside this project and there is "no" performance impact in linting "rules": {
"@typescript-eslint/no-unnecessary-type-assertion": "error"
},
"parserOptions": {
"sourceType": "module",
"project": [
"./packages/typescript-estree/tsconfig.json",
"./packages/parser/tsconfig.json",
"./packages/eslint-plugin-tslint/tsconfig.json"
],
"ecmaFeatures": {
"jsx": false
}
},rule itself has a lot of false positives (#187) but there is no issues with performance |
|
As an additional datapoint, I was looking into making the switch to ESLint for Outlook Web (currently ~20k TS files and ~1M lines of code). Running TSLint takes ~5-6 minutes, while running ESLint with this project took... well, I went to lunch, came back an hour later, and it was still running before I killed it. |
|
@MLoughry that would suggest you are using v1.x.x and/or the fallback behaviour of creating programs on the fly is being used (requires a flag in v2 and is not recommended). Please can you confirm the version you used? |
|
I was using version 2.3.0. I only started the work earlier this week, and pulled the latest. The ESLint configuration I used was as follows: {
"env": {
"browser": true,
"node": false
},
"parser": "@typescript-eslint/parser",
"parserOptions": {
"project": "tsconfig.noprojects.json",
"sourceType": "module"
},
"plugins": ["@typescript-eslint", "@typescript-eslint/tslint"],
"rules": {
"no-console": [
"error",
{
"allow": ["debug", "info", "log", "time", "timeEnd", "trace"]
}
],
"prefer-const": [
1,
{
"destructuring": "all"
}
],
"curly": "error",
"guard-for-in": "error",
"no-new-func": "error",
"no-labels": "error",
"no-caller": "error",
"no-constant-condition": "error",
"no-new-wrappers": "error",
"no-control-regex": "error",
"no-debugger": "error",
"no-delete-var": "error",
"no-duplicate-case": "error",
"no-eval": "error",
"no-multi-str": "error",
"no-octal": "error",
"no-regex-spaces": "error",
"no-throw-literal": "error",
"no-extra-bind": "error",
"use-isnan": "error",
"@typescript-eslint/tslint/config": [
"error",
{
"rulesDirectory": [
"./node_modules/tslint-microsoft-contrib",
"./node_modules/tslint-eslint-rules/dist/rules",
"./node_modules/owa-tslint-rules/lib",
"./node_modules/tslint-react/rules",
"./node_modules/tslint-react-hooks/dist"
],
"rules": {
"class-name": true,
"forbid-directory": [true, ["tests", "test"]],
"forbid-full-import": [true, "office-ui-fabric-react", "date-fns", "lodash"],
"forbid-import": [
true,
["mobx", "useStrict", "autorun", "action", "observable", "reaction"],
["dateformat", ""],
["satcheljs", "orchestratorAction"]
],
"interface-name": [true, "never-prefix"],
"no-banned-terms": true,
"no-document-domain": true,
"no-duplicate-imports": true,
"no-document-write": true,
"no-duplicate-variable": true,
"no-empty-interface": true,
"no-exec-script": true,
"no-for-in": true,
"no-http-string": [
true,
"http://www.example.com/?.*",
"http://www.examples.com/?.*",
"http://www.w3.org/.*",
"http://schema.org/.*",
"http://localhost(:\\d+)?/?.*",
"^http:$"
],
"no-inner-html": true,
"no-invalid-regexp": true,
"no-multiple-var-decl": true,
"no-namespace": true,
"no-reserved-keywords": false,
"no-string-based-set-immediate": false,
"no-string-based-set-interval": false,
"no-string-based-set-timeout": false,
"no-string-literal": true,
"promise-must-complete": true,
"react-no-dangerous-html": true,
"typedef": [true, "parameter"],
"use-named-parameter": true,
"variable-name": [true, "check-format", "allow-pascal-case"]
}
}
]
}
}I also tried eliminating all the TSLint rules and plugins (to run pure ESLint against Typescript files), but ran into similar performance issues. |
|
Our project isn't open source, so I can't share the exact repro (unless one of the maintainers is in Microsoft). Is there any suggestion on debugging or profiling this to try to figure out why the performance is so bad for our project? |
|
@MLoughry please could you reach out to @uniqueiniquity within Microsoft, I let him know about this |
|
@JoshuaKGoldberg Something smells fishy to me about tslint only taking 10s... |
|
Normal tsc build times are 7-8 seconds: ESLint is only linting the files it should, Output from
|
|
A better run command is It's not a problem with our tooling.
This is ofc hella slow. You have these two rules turned on (they're in the recommended set):
With these two rules turned on, Turning off those two rules brings the times down to be as fast as tslint: |
|
@MLoughry were you able to improve the performance of your codebase? Our codebase is of a similar size and running into the same performance issues while migrating from tslint to eslint. |
|
@suchanlee No, @uniqueiniquity was busy when I reached out to him and tabled the investigation, but I’ve not heard anything since. |
|
@MLoughry I see, then is your codebase still using tslint? |
|
Given that running eslint with types is significantly faster on each package of a monorepo vs. the whole monorepo itself, perhaps one solution would be for
Given that tslint performs significantly faster than typescript-eslint when type checking is turned on, and presumably tslint does not require calling |
I would contest this statement. Most of the cases of running significantly slower than TSLint that I've seen are because people are using slow lint rules (like the ones from It's also hard to exactly compare tslint and eslint 1:1. Most users that onboard onto ts-eslint add both ours and eslint's recommended configs, which turns on more rules than tslint's base recommended (54 vs 85).
This is incorrect. They use the The difference is that tslint owns their process e2e - from cli(/IDE plugin) through to rule implementation. We only own the parser and rule implementations; but the CLI and IDE plugins are all outside of our control. This has the side-effect of our parser having to be built to handle both the "one and done" CLI use case, and the "over and over" IDE plugin use case. Hence we use Also, in owning the IDE side of things they also get to leverage a TS language server plugin, which shares the programs, parsing etc with the TSServer that's running to provide types to the IDE, saving a bunch of time and memory. (Side note - there is a user that's actually built this for ts-eslint - https://github.com/Quramy/typescript-eslint-language-service) We haven't directly explored options like this yet because we're trying our hardest to not break from the standard ESLint environment - we don't want to introduce our own ESLint CLI or ESLint IDE Plugins, because it makes it harder for people to onboard, and is more things to maintain with a small maintainer base.
If you want complete type information - no. I'm not saying this project is performance perfect by any means - there are a number of problems that need to be solved. My current thinking is that many problems will more than likely have to be solved via PRing typescript core to make a better API for our use case (I'm really dreading hopping into that monster codebase...). |
|
@bradzacher Thanks so much for the detailed response! And I think it makes sense to try to not break away from core eslint as much as possible. And whilst I have no suggestions on how to do so, I do think that addressing performance issues or workarounds for these performance issues in things like IDE workflows may be something that might be of relatively high priority given that tslint is being deprecated in favor of typescript-eslint and more larger sized repositories will continue to migrate to typescript-eslint. I'll continue to do more testing on my end and try different things to see if we have workarounds and update this thread if I do! Thanks again for a great project 👍 |
|
Definitely, we are always looking to improve, but it's a problem of not having the data - we only know if there are tangible perf issues if people tell us, and people only tell us if they think it's a problem. The majority of users think "perf is good enough", because they operate at a small-medium scale. I've worked 1:1 with everyone that's reported perf issues so far to debug and figure out where the problem was. The common problems are being documented in the new onboarding docs I'm writing. In your case (copied the info you gave in #1192 to here for visibility):
Your project operates at a giant scale, and I'd hazard a guess that you're one of the largest multi-package monorepo style projects using our tooling. Now that we know that there are people with projects like this, we can look to solving problems. The biggest problem in your case would be No doubt fixing the duplication would also improve perf for all monorepos, due to all monorepos being interdependent, and sharing npm deps. |
|
We have an a fairly small Nx managed monorepo (~700 TS files in 60 libs, each lib with its own |
|
@demisx please file a separate issue and give us as much information as possible, so we can look into it. |
|
I know this issue seems a bit old, but since still open. |
|
My comment might not be directly related to benchmarking against - project: ['./tsconfig.json'],
+ project: ['./tsconfig.eslint.json', './packages/*/tsconfig.json'],proposed in doc for monorepo usage. Workaround that works for us is running eslint separately from each package by
parserOptions: {
- project: ['./tsconfig.json', './packages/*/tsconfig.json'],
+ project: ['./tsconfig.json'],
// points at each package tsconfig file, not the root one
- tsconfigRootDir: __dirname,
+ tsconfigRootDir: process.cwd(),
// becomes absolute_path/packages/foo/
},
- "lint": "eslint . --ext .js,.ts"
+ "lint": "lerna exec 'eslint \"src/**/*.{js,ts,tsx}\" --config \"../../.eslintrc.js\" ' --stream"
These changes got us to 70 seconds instead of 50 minutes. As I mentioner above this seem to be related to the
Config loading takes much more time than any other tasks. |
2 tasks
|
I am going to close this one. Improving performance is definitely important, but the need for a generic catch all "benchmark vs TSLint" issue is no longer the best way to do this and this thread contains a mixture of up to date and heavily outdated information. If you observe performance issues please report them for your specific case with as much detail as possible. As noted already, there is some guidance in https://github.com/typescript-eslint/typescript-eslint/blob/master/docs/getting-started/linting/MONOREPO.md for lerna-based monorepo projects. For @demisx @fbaba-nibtravel and others using Nx - the Nx lint builder needs to be updated, I have raised this with Victor already and it will be looked at soon. Thanks all! 🙏 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
First up, thanks so much for all of the work on this. I've long watched (and used) this project wondering if the community would eventually converge on TSLint or ESLint for checking TypeScript code and am really excited to see the recent progress on this project.
Once this project hits 1.0 it would be interesting to benchmark it against TSLint on some largish codebases. This would not only help people in deciding which to use (although speed is only a minor concern IMO, number and features of rules and community adoption would be more important to me) but it could also potentially help find bottlenecks in different rules.
The text was updated successfully, but these errors were encountered: