Skip to content

Conversation

@notJoon
Copy link
Member

@notJoon notJoon commented Nov 18, 2025

Description

This pull request introduces the instrumentation-first profiling architecture. The VM now emits structured events (CPU/gas samples, allocation events, and line samples) through a instrumentation.Sink interface, and the profiler consumes those events without depending on VM internals.

docs (draft): https://gist.github.com/notJoon/de3a4e4fceeedd25bddcecef289be547

Key Changes

Instrumentation Layer

  • Added instrumentation package with SampleContext, FrameSnapshot, AllocationEvent, LineSample, and Sink.
  • Machine accepts an optional sink via MachineOptions. When nil, the VM runs with zero additional work (only a nil-check).
  • Allocation and line-level hooks now emit events only when a sink advertises interest.

Profiler Core

  • The profiler implements the sink interface, maintaining:
    • Canonical frame table (frame interning → stable FrameIDs).
    • Incremental call-tree aggregation.
    • Function statistics (call count, self/total cycles/gas, allocations).
    • Per-function line tables and file-level line stats.
  • Sampling updates these aggregates incrementally; final profiles are produced via Profile.WriteFormat.

CLI / Test Harness Integration

  • ProfileConfig wires sinks into every test VM, enables gas meters or allocators as needed, and exposes new flags:
    • -profile master switch.
    • -profile-type, -profile-format, -profile-sample-rate, -profile-output, -profile-stdout.
    • -profile-interactive (launch shell) and -profile-line (enable line-level sampling).
  • Interactive shell mirrors pprof commands (top, list, calltree, json, text, clear, exit), reusing the captured profile without rerunning tests.

@Gno2D2
Copy link
Collaborator

Gno2D2 commented Nov 18, 2025

🛠 PR Checks Summary

🔴 Pending initial approval by a review team member, or review from tech-staff

Manual Checks (for Reviewers):
  • IGNORE the bot requirements for this PR (force green CI check)
Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

🟢 Maintainers must be able to edit this pull request (more info)
🔴 Pending initial approval by a review team member, or review from tech-staff

☑️ Contributor Actions:
  1. Fix any issues flagged by automated checks.
  2. Follow the Contributor Checklist to ensure your PR is ready for review.
    • Add new tests, or document why they are unnecessary.
    • Provide clear examples/screenshots, if necessary.
    • Update documentation, if required.
    • Ensure no breaking changes, or include BREAKING CHANGE notes.
    • Link related issues/PRs, where applicable.
☑️ Reviewer Actions:
  1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.
📚 Resources:
Debug
Automated Checks
Maintainers must be able to edit this pull request (more info)

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 The pull request was created from a fork (head branch repo: notJoon/gno-core)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this pull request

Pending initial approval by a review team member, or review from tech-staff

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 Not (🔴 Pull request author is a member of the team: tech-staff)

Then

🔴 Requirement not satisfied
└── 🔴 If
    ├── 🔴 Condition
    │   └── 🔴 Or
    │       ├── 🔴 At least one of these user(s) reviewed the pull request: [jefft0 leohhhn n0izn0iz notJoon omarsy x1unix] (with state "APPROVED")
    │       ├── 🔴 At least 1 user(s) of the team tech-staff reviewed pull request
    │       └── 🔴 This pull request is a draft
    └── 🔴 Else
        └── 🔴 And
            ├── 🟢 This label is applied to pull request: review/triage-pending
            └── 🔴 On no pull request

Manual Checks
**IGNORE** the bot requirements for this PR (force green CI check)

If

🟢 Condition met
└── 🟢 On every pull request

Can be checked by

  • Any user with comment edit permission

Previously, allocation events were emitted without proper call stack
context, making it difficult to attribute memory consumption to actual
source functions. This change ensures allocation profiling captures
the complete execution context at allocation time.

Changes:

* Profiler: RecordAlloc now increments CallCount for allocation events
  - Allocation statistics now appear correctly in toplists and call trees
  - Functions allocating memory are tracked with proper call counts
  - Added TestProfilerRecordAllocUsesCallStack to verify allocations
    derive function names, bytes, and object counts from the top frame
    rather than synthetic "<allocation>" markers

* Interactive shell: Memory mode toplists now display allocation metrics
  - Both flat and cumulative columns show bytes allocated
  - Appropriate labels and percentages reflect memory-specific semantics
  - Differs from CPU/gas modes where cumulative includes nested calls

* Allocator sink wiring: Introduced allocationStackInjector adapter
  - Wraps allocator sinks to capture machine frames when events lack them
  - Ensures all memory events inherit real call stacks from VM state
  - Added TestAllocationStackInjectorAddsMachineFrames to verify
    stack injection behavior
Profiling a package now captures both standard tests and filetests, and momory mode no longer drops data when `MAXALLOC` is unspecified.

- Updated filetest codebase to provision machines for profiling: when a memory profile is requested and no `MAXALLOC` is set, we now install an unlimited allocator so allocation events reach the profiler. we also attach the `ProfileConfig` sink to filetest mechaines, putting `*_filetest.gno` executions on par with `_test.gno`.

- Added `provisionFiletestAllocator` helper and unit tests in `gnovm/pkg/test/filetest_test.go` to ensure we only create allocators when necessary and leave existing ones untouched.
@notJoon notJoon marked this pull request as ready for review November 20, 2025 09:26
@Gno2D2 Gno2D2 added the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Nov 20, 2025
@Kouteki Kouteki moved this from Triage to In Review in 🧙‍♂️gno.land core team Nov 20, 2025
- Sampling only happend in `OpCall`, missing work in loop/other ops
- Move `maybeEmitSample` into `incrCPU` so every opcode can trigger samples
- Test: ensure `maybeEmitSample` fires when CPU cycles are incremented (profile sink receives a sample)
Profiling now runs as a single session across gno test package loops,
so profile data and the interactive shell cover all packages instead
of just the last one.

## Changes

- add `Start`/`Stop` lifecycle helpers on `ProfileConfig`. Test now only initializes/finalizes profiling when it owns the session, skipping per-package resets when a session is already active.

- `execTest` starts profiling once, ensures cleanup even on early returns, and finalizes before `maybeStartProfileShell` so the shell sees the aggregated profile.
Profiling now carries machine identity through instrumentation, keeps per-machine baselines, attributes allocations into call trees/line stats, and guards against accidental profiler restarts.

- add `MachineID` plumbing on samples/allocations/line samples, propagate through adapter/machine with unsafe pointer IDs

- per-machine baselines with ensureBaseline/optional Identity, StartProfiling returns early when already active, allocations now flow into call tree and line stats with bytes/objects tracked, call tree export/sorting honors memory profiles, function sorting updated for memory, new helpers for allocation line stats.
@Kouteki Kouteki requested a review from jaekwon November 29, 2025 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📦 🤖 gnovm Issues or PRs gnovm related review/triage-pending PRs opened by external contributors that are waiting for the 1st review

Projects

Status: No status
Status: In Review

Development

Successfully merging this pull request may close these issues.

2 participants