Broken `test/v8-updates/test-linux-perf-logger`

[targos]

/cc @lukealbao

Seen in https://github.com/nodejs/node/pull/50115 Test added in https://github.com/nodejs/node/pull/50352

https://ci.nodejs.org/job/node-test-commit-v8-linux/5743/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/consoleFull

12:00:23 not ok 2 v8-updates/test-linux-perf-logger
12:00:23   ---
12:00:23   duration_ms: 714.05100
12:00:23   severity: fail
12:00:23   exitcode: 1
12:00:23   stack: |-
12:00:23     node:assert:126
12:00:23       throw new AssertionError(obj);
12:00:23       ^
12:00:23     
12:00:23     AssertionError [ERR_ASSERTION]: 2 tests failed
12:00:23     
12:00:23     [ERROR 1] --perf-basic-prof compiled
12:00:23     Errors:
12:00:23     1. Expected to match /[a-fA-F0-9]+ [a-fA-F0-9]+.*:\*functionOne .+\/linux-perf-logger.js/
12:00:23     2. Expected to match /[a-fA-F0-9]+ [a-fA-F0-9]+.*:\*functionTwo .+\/linux-perf-logger.js/
12:00:23     Perf map content:
...
12:00:24     </end perf map content>
12:00:24     
12:00:24         at runSuite (/home/iojs/build/workspace/node-test-commit-v8-linux/test/v8-updates/test-linux-perf-logger.js:145:10)
12:00:24         at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-v8-linux/test/v8-updates/test-linux-perf-logger.js:148:1)
12:00:24         at Module._compile (node:internal/modules/cjs/loader:1378:14)
12:00:24         at Module._extensions..js (node:internal/modules/cjs/loader:1437:10)
12:00:24         at Module.load (node:internal/modules/cjs/loader:1212:32)
12:00:24         at Module._load (node:internal/modules/cjs/loader:1028:12)
12:00:24         at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:142:12)
12:00:24         at node:internal/main/run_main_module:28:49 {
12:00:24       generatedMessage: false,
12:00:24       code: 'ERR_ASSERTION',
12:00:24       actual: 2,
12:00:24       expected: 0,
12:00:24       operator: 'strictEqual'
12:00:24     }
12:00:24     
12:00:24     Node.js v22.0.0-pre
12:00:24   ...
12:00:24 
12:00:24 Failed tests:
12:00:24 out/Release/node /home/iojs/build/workspace/node-test-commit-v8-linux/test/v8-updates/test-linux-perf-logger.js

[lukealbao]

Hi @targos sorry I haven't followed up on this -- this happened while I was out of pocket, and I thought I'd seen it get resolved before I returned. In particular, I missed the comment that the #50352 landed but wasn't actually being exercised in CI. I see that conversations are still happening around this test (cf. https://github.com/nodejs/build/issues/3645).

I'm unable to view CI runs. Has auth around that changed? I'd like to help if I can. I believe @richardlau is right that nodejs/node@515b007 would need to be backported to the v18.x branch.

Separately but relatedly, it looks like more recent v8 versions are causing the compiled test cases to fail due to the fact that they don't exceed the newly added ~invocation-count-for-turbofan~ minimum-invocations-before-optimization v8 option's default. Setting that to zero in the child process options make these cases pass for me when running on the main branch. Without view access to CI, I'm just taking a stab, but I suspect that's what's going on with the test-perf tests, as well. As I mentioned elsewhere, #50352 is just a port of that test suite, without the external perf dependency.

So, summing up my understanding of how to get this test suite active in all maintained branches:

• v18.x needs to have the changes from 515b007 backported. That will prevent the child process from exiting due to a bad v8 option.
• all others will need to add new invocation* count-based options to force JIT. Previously, that seems to have been accomplished simply by the --always-<jit> option.

Would it be helpful for me to submit a PR for the above?

[targos]

Yes, it would be very helpful!

Note that the test was just skipped in CI yesterday so a fixing PR would have to re-enable it.

I'm not sure why you don't have access to CI. Do you get an error message or something when you try to access it?

[lukealbao]

I tried logging out and in before, but now that I look at my notifications, I see this warning about the git plugin needing to be patched, so maybe it's that?

[targos]

Ah no, it's an old run that's not available anymore. Try https://ci.nodejs.org/job/node-test-commit-v8-linux/5954/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/console

[lukealbao]

Thanks @targos! Re the first bullet point: I think I was mistaken. On closer look, looks like nodejs/build#3645 was running on v21.x.

• Do I understand correctly that adding the test-linux-perf-logger file to v18 is a no-go by virtue of it being in maintenance mode?
• Should I keep an eye out to do anything special about backporting fixes into other branches? I see that 52869 is labelled as don't land on v20.
• AFAICT, CI for 52869 is failing because of windows heap exhaustion, but please LMK if there's something material I can help with around this issue.

[richardlau]

I ran a V8 CI for v18.x-staging -- it failed because of test-linux-perf: https://ci.nodejs.org/job/node-test-commit-v8-linux/5966/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/console While v18.x is in maintenance, we do still occassionally need to land V8 backports (see https://github.com/nodejs/node/pull/52337 for example) so it would be really useful to be able to get the V8 CI passing again, whether that would be fixing test-linux-perf or landing test-linux-perf-logger as a replacement.