Skip to content

fix: emit perf maps when not parsing the full inlining logs from V8#85

Merged
GuillaumeLagrange merged 1 commit into
mainfrom
cod-2987-e2e-tests-show-no-flamegraph-for-vitest
Jun 24, 2026
Merged

fix: emit perf maps when not parsing the full inlining logs from V8#85
GuillaumeLagrange merged 1 commit into
mainfrom
cod-2987-e2e-tests-show-no-flamegraph-for-vitest

Conversation

@GuillaumeLagrange

@GuillaumeLagrange GuillaumeLagrange commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@greptile-apps

greptile-apps Bot commented Jun 24, 2026

Copy link
Copy Markdown

Greptile Summary

This PR restores the --perf-basic-prof V8 flag for walltime mode when CODSPEED_V8_LOG is not set, and removes the now-released alpha runner version pin from the codspeed-walltime CI job.

  • introspection.ts: Adds an else branch so that when CODSPEED_V8_LOG is unset in walltime mode, --perf-basic-prof is pushed — this writes a /tmp/perf-<pid>.map file that linux perf / samply uses for JS symbol resolution alongside the always-emitted jitdump (--perf-prof).
  • codspeed.yml: Drops the runner-version: v4.17.7-alpha.2 pin and its associated TODO comment from the codspeed-walltime job, confirming the runner is now generally available.

Confidence Score: 5/5

Safe to merge — restores a flag that was accidentally removed and cleans up a released-runner pin.

Both changes are narrow and correct: the --perf-basic-prof fallback is only active in walltime mode when CODSPEED_V8_LOG is absent, and it complements (not conflicts with) the always-present --perf-prof jitdump flag. The CI change simply removes a version pin whose TODO comment explicitly marked it for removal once the runner shipped.

No files require special attention.

Important Files Changed

Filename Overview
packages/core/src/introspection.ts Adds --perf-basic-prof fallback in walltime mode when CODSPEED_V8_LOG is unset, restoring the perf.map output path for linux perf/samply symbol resolution.
.github/workflows/codspeed.yml Removes the runner-version: v4.17.7-alpha.2 pin (and its TODO comment) from the codspeed-walltime job, indicating the runner has been officially released.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[getV8Flags - walltime mode] --> B["push --perf-prof\n(always emit jitdump)"]
    B --> C{CODSPEED_V8_LOG\nenv var set?}
    C -- Yes --> D["push --log-code\n--no-log-source-code\n--no-logfile-per-isolate\n--logfile=<dir>/codspeed-v8-%p.log"]
    C -- No --> E["push --perf-basic-prof\n(write /tmp/perf-PID.map)"]
    D --> F[Return flags]
    E --> F
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[getV8Flags - walltime mode] --> B["push --perf-prof\n(always emit jitdump)"]
    B --> C{CODSPEED_V8_LOG\nenv var set?}
    C -- Yes --> D["push --log-code\n--no-log-source-code\n--no-logfile-per-isolate\n--logfile=<dir>/codspeed-v8-%p.log"]
    C -- No --> E["push --perf-basic-prof\n(write /tmp/perf-PID.map)"]
    D --> F[Return flags]
    E --> F
Loading

Reviews (2): Last reviewed commit: "fix: have node output perf maps when not..." | Re-trigger Greptile

Comment thread packages/core/src/index.ts Outdated
@codspeed-hq

codspeed-hq Bot commented Jun 24, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 58.32%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 4 improved benchmarks
❌ 7 regressed benchmarks
✅ 227 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Memory short body 536 B 3,656 B -85.34%
Simulation test_recursive_fibo_10 140.2 µs 275.4 µs -49.1%
WallTime short body 2.4 µs 2.8 µs -15.52%
WallTime short body 2.3 µs 2.6 µs -12.33%
WallTime short body 2.3 µs 2.6 µs -12.09%
WallTime short body 2.3 µs 2.6 µs -11.27%
WallTime test_recursive_cached_fibo_10 1 µs 1.1 µs -10.53%
Memory fibo 15 5,120 B 5 B ×1,000
Simulation recursive fibo 10 316.7 µs 138.1 µs ×2.3
Memory wait 1ms 10 B 7 B +42.86%
WallTime test_iterative_fibo_10 276 ns 228 ns +21.05%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing cod-2987-e2e-tests-show-no-flamegraph-for-vitest (c88994c) with main (36834e8)

Open in CodSpeed

@GuillaumeLagrange GuillaumeLagrange force-pushed the cod-2987-e2e-tests-show-no-flamegraph-for-vitest branch from 205bafe to c88994c Compare June 24, 2026 08:58
@GuillaumeLagrange GuillaumeLagrange changed the title fix: re-enable linux perf listener for walltime fix: emit perf maps when not parsing the full inlining logs from V8 Jun 24, 2026
@GuillaumeLagrange GuillaumeLagrange merged commit c88994c into main Jun 24, 2026
30 of 31 checks passed
@GuillaumeLagrange GuillaumeLagrange deleted the cod-2987-e2e-tests-show-no-flamegraph-for-vitest branch June 24, 2026 09:06
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

Successfully merging this pull request may close these issues.

2 participants