Skip to content

WIP: fix(gnovm): size bound methods by the wrapper, not the wrapped func's package#5852

Draft
ltzmaxwell wants to merge 5 commits into
gnolang:masterfrom
ltzmaxwell:fix/bmv-shallowsize-uverse
Draft

WIP: fix(gnovm): size bound methods by the wrapper, not the wrapped func's package#5852
ltzmaxwell wants to merge 5 commits into
gnolang:masterfrom
ltzmaxwell:fix/bmv-shallowsize-uverse

Conversation

@ltzmaxwell

Copy link
Copy Markdown
Contributor

WIP / stacked on #5737. Based on that branch, so the diff below includes #5737's commits — only the final commit (BoundMethodValue.GetShallowSize) is in scope here. Will rebase onto master once #5737 merges. Do not review until then.

  • BoundMethodValue.GetShallowSize returned 0 when the wrapped Func's PkgPath was ".uverse", under-counting a real, dynamically-allocated bmv wrapper of a uverse method — e.g. g := addr.String, a value-method on the concrete uverse address type — by allocBoundMethod (248 bytes; confirmed via runtime.MemStats).
  • A bmv is a runtime composite with no PkgPath of its own (unlike PackageValue/Block/FuncValue, which correctly key on their own package); its size is now always allocBoundMethod, with counting governed by GC/persistence reachability.
  • Hard-fork: observably changes GC/persist sizing.
  • Test: TestBoundMethodValueGetShallowSize pins the size for uverse-func / lazy (Func==nil) / normal bmvs.

@Gno2D2

Gno2D2 commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

🛠 PR Checks Summary

All Automated Checks passed. ✅

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)

☑️ 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: ltzmaxwell/gno)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this 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

…values

Interface-bound method values now resolve their concrete method and receiver
at call time (deref, value snapshot, nil panic, embedded-field re-read and
dynamic re-dispatch all match Go), instead of binding eagerly. The call-time
dispatch is charged via OpCPULazyBoundResolve. Hard-fork: BoundMethodValue
gains a persisted Method field (wire/merkle/gas change).
… package

GetShallowSize returned 0 when Func.PkgPath==".uverse", under-counting real, dynamically-allocated wrappers of uverse methods (e.g. `g := addr.String`, a value-method on the concrete uverse address type) by allocBoundMethod; it is now always allocBoundMethod, with counting governed by GC/persistence reachability. Hard-fork: observably changes GC/persist sizing (confirmed via runtime.MemStats: +248 bytes).
@ltzmaxwell ltzmaxwell force-pushed the fix/bmv-shallowsize-uverse branch from 9542bc3 to 5761f1a Compare June 24, 2026 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📦 ⛰️ gno.land Issues or PRs gno.land package related 📦 🤖 gnovm Issues or PRs gnovm related

Projects

Development

Successfully merging this pull request may close these issues.

2 participants