-
Notifications
You must be signed in to change notification settings - Fork 107
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
Significantly increased memory usage in 3.13.0 #1246
Comments
@markrian thanks for the detailed report, and sorry for this (: this should be fixed in |
@B2o5T Thanks for the fast reply and fix! Unfortunately, I'm still seeing memory issues with 3.13.1. See this job failure, which ran against 3.13.1.
Oh, I can't reopen this 😅 |
do you have a failure locally as well? Update:
You are right, It's due to this PR. I tried to fix it by this check https://github.com/B2o5T/graphql-eslint/blob/b1b5ec7da638a58f96295c885476c16f1fc39963/packages/plugin/src/cache.ts#L23 but seems it's not enough for you. |
I ran some tests locally, and I found 3.13.0 uses around 1.5GiB compared to 1.4GiB for 3.12.0 and 3.13.1 for that particular job. That doesn't sound like much, but maybe that's just enough to get killed in our CI (I'm not sure exactly what resource limits are like there). As far as I can tell, yes, Given that I see effectively identical memory usage between 3.12.0 and 3.13.1 (but not 3.13.0) locally, I wonder if there's some Ci-related caching issue going on. I'll have a look into this. I don't think this would help this particular case, but looking at the diff --git a/packages/plugin/src/cache.ts b/packages/plugin/src/cache.ts
index 915c391..748af55 100644
--- a/packages/plugin/src/cache.ts
+++ b/packages/plugin/src/cache.ts
@@ -24,6 +24,9 @@ export class ModuleCache<T, K = any> {
process.hrtime(lastSeen)[0] < settings.lifetime
) {
return result;
+ } else {
+ // evict stale result
+ this.map.delete(cacheKey);
}
}
} Edit: Ah, no, I see it's getting overwritten elsewhere anyway. |
Between |
Sadly it still OOMs. It's frustrating that I can't replicate this locally 😩 I think I might just set |
Huh, even with I'm going to have to put this aside for now and stay on 3.12.0 until I have more time to look into this. |
I will close this issue due to no activity, will reopen it if it still exist for somebody |
@vitallium managed to figure out what the problem was! Our configuration had this: operations:
- '{,ee/,jh/}app/**/*.graphql' And by enabling logging, we saw we got a lot of cache misses like this:
For whatever reason, the operations: '{,ee/,jh/}app/**/*.graphql' That |
@markrian thanks for the detailed report, it’s very helpful. I thought the same reference for the If anyone wants to deep dive feel free until I will have some time to take a look deeply into why it occurs |
@markrian how can I generate |
@markrian seems the problem is with how you run eslint https://gitlab.com/gitlab-org/gitlab/-/blob/master/package.json#L22 "lint:eslint:all": "node scripts/frontend/eslint.js ." and your custom script https://gitlab.com/gitlab-org/gitlab/-/blob/master/scripts/frontend/eslint.js const { spawn } = require('child_process');
const runEslint = () => {
const [, , ...args] = process.argv;
const child = spawn(`yarn`, ['internal:eslint', ...args], {
stdio: 'inherit',
});
child.on('exit', (code) => {
process.exitCode = code;
if (code === 0) {
return;
}
console.log(`
If you are seeing @graphql-eslint offences, the local GraphQL schema dump might be outdated.
Consider updating it by running \`./scripts/dump_graphql_schema\`.
`);
});
};
runEslint(); any real benefit of it? @vitallium I have read your investigation
But I don't think is true ( |
I'm running into memory issues as well as of 3.13.0 and haven't been able to upgrade this library since, as I can't get ESLint to run without crashing, but I haven't been able to debug and/or dig into it further so have nothing useful to contribute to this issue. Fwiw, no custom scripts here, just running ESLint directly. |
@pleunv try to update to the latest version and run your lint command with debug |
Locally I can't seem to get it to crash at this time, however, it does still consistently crash on CI:
I do notice a massive perf regression when running locally. v3.12.0:
v3.19.3:
Both instances are run without a cache. |
@pleunv try remove if it fixes your issue I'll try to remove all unrelated code from production build |
Oh wow... Yeah, that was it! Back to normal speeds now. Not even sure why it was there, probably a remnant from the |
Issue workflow progress
Progress of the issue based on the Contributor Workflow
Describe the bug
3.13.0 has increased memory consumption over 3.12.0, to the point that in CI the process is OOM-killed when run against the GitLab repository.
Using 3.12.0, the job passes reliably, while on 3.13.0 it fails reliably.
To Reproduce
Steps to reproduce the behavior:
(Apologies that this isn't a minimal reproduction. If/when I have time, I'll make one and post it here.)
For now:
Expected behavior
Memory usage shouldn't be significantly higher than with 3.12.0.
Environment:
@graphql-eslint/eslint-plugin
: 3.13.0Additional context
I suspect that #1222 might be implicated, since it touches some caching-related stuff and was the most significant change between 3.12.0 and 3.13.0. But that's just a hunch.
The text was updated successfully, but these errors were encountered: