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

vitest hangs without typeCast #2206

Closed
Cellule opened this issue Sep 12, 2023 · 7 comments
Closed

vitest hangs without typeCast #2206

Cellule opened this issue Sep 12, 2023 · 7 comments

Comments

@Cellule
Copy link

Cellule commented Sep 12, 2023

Okay I know this sounds super weird, bear with me.
We just started migrating from jest to vitest in our codebase.
The machines would just hang in the CI when running them. I couldn't for the life of me figure out why.
After a lot of trial and error I narrowed it down to 1 thing.

If I pass a typeCast (empty, just calling next())=> Tests run fine everything is good
If I don't pass a typeCast => Tests hangs in CI and I can see a good extra consistent 10s additional teardown (from 7s to 17s to run a few files)

I suspect vitest doesn't like something here

} else {
const encodingExpr = `fields[${i}].encoding`;
const readCode = readCodeFor(
fields[i].columnType,
fields[i].characterSet,
encodingExpr,
config,
options
);
if (typeof options.typeCast === 'function') {
parserFn(`${lvalue} = options.typeCast(this.wrap${i}, function() { return ${readCode} });`);
} else {
parserFn(`${lvalue} = ${readCode};`);
}

This is likely a vitest specific issue, but any insights I could get here would go a long way to help me open up an issue on their side to investigate.
Note: We're still on 2.3.3, but I confirmed the issue still happens on latest 3.6.1

For reference here's the parser with typecast and without

With typeCast

(function anonymous(wrap
) {
return ((function () {
  return class TextRow {
    constructor(fields) {
      const _this = this;
      for(let i=0; i<fields.length; ++i) {
        this[`wrap${i}`] = wrap(fields[i], _this);
      }
    }
    next(packet, fields, options) {
      this.packet = packet;
      const result = {};
      // "id": LONG
      result["id"] = options.typeCast(this.wrap0, function() { return packet.parseLengthCodedIntNoBigCheck() });
      // "auth_type": STRING
      result["auth_type"] = options.typeCast(this.wrap1, function() { return packet.readLengthCodedString(fields[1].encoding) });
      // "phone_number": VAR_STRING
      result["phone_number"] = options.typeCast(this.wrap2, function() { return packet.readLengthCodedString(fields[2].encoding) });
      // "email": VAR_STRING
      result["email"] = options.typeCast(this.wrap3, function() { return packet.readLengthCodedString(fields[3].encoding) });
      // "first_name": VAR_STRING
      result["first_name"] = options.typeCast(this.wrap4, function() { return packet.readLengthCodedString(fields[4].encoding) });
      // "last_name": VAR_STRING
      result["last_name"] = options.typeCast(this.wrap5, function() { return packet.readLengthCodedString(fields[5].encoding) });
      // "img": VAR_STRING
      result["img"] = options.typeCast(this.wrap6, function() { return packet.readLengthCodedString(fields[6].encoding) });
      // "created_at": TIMESTAMP
      result["created_at"] = options.typeCast(this.wrap7, function() { return packet.parseDateTime('local') });
      // "updated_at": TIMESTAMP
      result["updated_at"] = options.typeCast(this.wrap8, function() { return packet.parseDateTime('local') });
      // "activated_at": DATETIME
      result["activated_at"] = options.typeCast(this.wrap9, function() { return packet.parseDateTime('local') });
      // "last_visited_at": DATETIME
      result["last_visited_at"] = options.typeCast(this.wrap10, function() { return packet.parseDateTime('local') });
      // "last_invite_sent_at": DATETIME
      result["last_invite_sent_at"] = options.typeCast(this.wrap11, function() { return packet.parseDateTime('local') });
      return result;
    }
  };
})())
})

Without typeCast

(function anonymous(
) {
return ((function () {
  return class TextRow {
    constructor(fields) {
    }
    next(packet, fields, options) {
      this.packet = packet;
      const result = {};
      // "id": LONG
      result["id"] = packet.parseLengthCodedIntNoBigCheck();
      // "auth_type": STRING
      result["auth_type"] = packet.readLengthCodedString(fields[1].encoding);
      // "phone_number": VAR_STRING
      result["phone_number"] = packet.readLengthCodedString(fields[2].encoding);
      // "email": VAR_STRING
      result["email"] = packet.readLengthCodedString(fields[3].encoding);
      // "first_name": VAR_STRING
      result["first_name"] = packet.readLengthCodedString(fields[4].encoding);
      // "last_name": VAR_STRING
      result["last_name"] = packet.readLengthCodedString(fields[5].encoding);
      // "img": VAR_STRING
      result["img"] = packet.readLengthCodedString(fields[6].encoding);
      // "created_at": TIMESTAMP
      result["created_at"] = packet.parseDateTime('local');
      // "updated_at": TIMESTAMP
      result["updated_at"] = packet.parseDateTime('local');
      // "activated_at": DATETIME
      result["activated_at"] = packet.parseDateTime('local');
      // "last_visited_at": DATETIME
      result["last_visited_at"] = packet.parseDateTime('local');
      // "last_invite_sent_at": DATETIME
      result["last_invite_sent_at"] = packet.parseDateTime('local');
      return result;
    }
  };
})())
})
@sidorares
Copy link
Owner

hm, strange indeed. Could you try to reduce the issue to a smallest self contained example? That way I can try to debug it locally

@Cellule
Copy link
Author

Cellule commented Sep 12, 2023

I'll give it a try, our project is pretty big so it's hard to know exactly what's causing the issue. I suspect maybe 1 particular data type of one of our MySQL table is at fault so I'll try to reduce that then see if I can get a minimal repro in a public repo

@Cellule
Copy link
Author

Cellule commented Sep 12, 2023

Alright, so I think I managed to repro in a more isolated environment.
Somehow in that repo typeCast doesn't seem to make a difference like it does in my project, but maybe the issue is related.
I managed to repro in node without vitest, let me know if this helps.

https://github.com/Cellule/mysql2-vitest-hang-repro
You'll need to adjust the config to a mysql database in src/db.js

Then you can run npm run cli-slow
You should notice a long time to teardown the process somehow vs the npm run cli-quick one

image

@sidorares
Copy link
Owner

can't reproduce, both run cli-slow and cli-quick finish in a reasonable short time:

laplace@Andreys-MacBook-Pro mysql2-vitest-hang-repro % npm run cli-quick

> [email protected] cli-quick
> node src/hang-cli.js 200

Starting
execution: 383.165ms
Tearing Down
done: 4.221ms
laplace@Andreys-MacBook-Pro mysql2-vitest-hang-repro % npm run cli-slow 

> [email protected] cli-slow
> node src/hang-cli.js 250

Starting
execution: 458.581ms
Tearing Down
done: 2.396ms
laplace@Andreys-MacBook-Pro mysql2-vitest-hang-repro % npm run cli-slow

> [email protected] cli-slow
> node src/hang-cli.js 250

Starting
execution: 481.298ms
Tearing Down
done: 2.105ms

what's your node version? I wonder if this is related to #2090
Can you try running your example with --noopt ? ( see #2090 (comment) )

@Cellule
Copy link
Author

Cellule commented Sep 13, 2023

So I was running with node 16.10
I repro'd on node 18.17.1
I couldn't repro as-is on node 20.6.1 but if I increase the iteration count to about 500 it reproes there too

--noopt and --max-opt=2 seems to fix the issue even at 10000 iterations

image

I have also confirmed that --noopt solves the issue on my project as well

So I guess this issue is a duplicate of #2090
You can close as duplicate in this case and I'll track the other issue

@sidorares
Copy link
Owner

sidorares commented Sep 14, 2023

let's keep #2090 to track this issue

The problem is caused by some V8 optimiser quirks - see https://bugs.chromium.org/p/v8/issues/detail?id=14133

Possible solutions:

@sidorares
Copy link
Owner

sidorares commented Sep 14, 2023

@Cellule if you have capacity feel free to work on last 2 options. Unfortunately I don't have a lot of free time right now so to ETAs from my side, but happy to guide / review PRs if someone wants to help

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

2 participants