v8-updates/test-linux-perf no longer works with recent V8 versions
#50079
Comments
targos
added
v8 engine
|
FYI @nodejs/diagnostics |
|
It seems to be similar to what I saw in #48862 (comment) although before I was unable to reproduce it locally and it only showed up in the benchmark machine, so I am not sure if it's the flag being broken, or it's the perf version on that machine no longer works with the output. One workaround might be that instead of looking into the perf script output, we only check the map file to see if the mapping is emitted correctly (though I am not sure if we can check the correctness of map enough to ensure that Were you able to reproduce this locally though? |
|
On the problematic machine: I don't know how perf was installed there, but maybe using a lower version that matches the kernel version could make a difference... |
It should have been via the Ansible scripts in https://github.com/nodejs/build/blob/main/ansible/roles/linux-perf/tasks/main.yml. |
|
It seems to be pulling perf from the tip of the tree at the time of installation, I think we should install the one matching the kernel version on that machine instead. |
|
👋 I surely don't have all the infra context here, but I think #50352 may allow these to be removed altogether. |
Refs: nodejs#50079 PR-URL: nodejs#49639 Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
|
I am not a ansible ninja so don't know how to update https://github.com/nodejs/build/blob/main/ansible/roles/linux-perf/tasks/main.yml, but anyway I think the right way to download the right source code when building perf is: #!/bin/bash
kernel_version=$(uname -r)
major_minor_version=$(echo "$kernel_version" | awk -F. '{print $1"."$2}')
version=$major_minor_version
echo "Kernel version: $version"
git clone --depth 1 --branch v$version git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git |
|
Test still fails with a matching |
|
From the log it looks like V8 just decides to compile the target functions, not interpreting them at all (perhaps related to recent Maglev changes too). I wonder if we should just check if we have some interpreted functions and and call it a day. It's probably just not sound to assume that these functions must be interprted. |
Refs: nodejs#50079 PR-URL: nodejs#49639 Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
|
This test is currently skipped. It's |
Refs: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/5600/console
The text was updated successfully, but these errors were encountered: