node/app/component: use []*component for relations storage#20585
Merged
Conversation
Change the internal relations type from []relation (interface slice) to []*component. The relation interface is still used as the public parameter type for Add/Remove/Contains/WithDependencies/WithDependent — callers pass any relation and we unwrap to *component at the boundary via asComponent. Why: - Pointer identity comparison becomes a direct *component == *component, eliminating the uintptr(unsafe.Pointer(...)) dance via the relationPtr helper. - Avoids the interface-header allocation that happens when converting *component to relation for storage. - cmp/samePtr now take *component directly, making the intent explicit. Net: 32 insertions, 20 deletions. All node/app tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 tasks
mh0lt
added a commit
that referenced
this pull request
Apr 15, 2026
… commit
The glamsterdam suite was tracking upstream
ethpandaops/ethereum-package@main unpinned, while other suites
(regular, pectra) pin to a specific version. Upstream commit 835dd9b
("feat: support gpu ere prover in zkboost", 2026-04-15) introduced an
undefined GpuConfig reference in zkboost_launcher.star:272, breaking
every Erigon PR that ran glamsterdam from that point on — regardless
of what the PR changed. Six unrelated PRs (#20471, #20472, #20526,
#20583, #20584, #20585) all failed identically.
Pin to e07503d16b (2026-04-13, the commit before the break) to stop
the bleeding. This was the last state under which our main successfully
ran glamsterdam. Revisit once upstream stabilises, or switch to a
tagged release that supports gloas_fork_epoch / fulu_fork_epoch which
our glamsterdam.io config requires.
2 tasks
AskAlexSharov
approved these changes
Apr 16, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the component framework’s internal dependency/dependent “relations” storage to use concrete []*component rather than an interface slice, reducing allocations and simplifying identity comparisons inside node/app/component.
Changes:
- Change internal
relationsfrom[]relationto[]*component. - Update relation set operations (
Add/Remove/Contains) to unwraprelation→*componentat the API boundary and compare pointers directly. - Update
WithDependencies/WithDependentoptions to store unwrapped*componentvalues.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Apr 16, 2026
## Summary Follow-up PR addressing seven unresolved review comments from #20450 (componentization framework). All fixes are focused and localized. ### Fixes included - **`event/eventbus.go`**: `HasCallback` now uses `RLock`/`RUnlock` instead of `Lock`/`Unlock` — it only reads `handlerMap` and a full write lock blocks concurrent `Publish`/`Subscribe` unnecessarily. - **`component/componentdomain.go`**: Guard nil dereference of `opts.execPoolSize` in the root-domain fallback path. Also return `false` from the `WithDependentDomain` option callback when the passed `ComponentDomain` is not a `*componentDomain`, instead of panicking on the type assertion. - **`event/managedeventbus.go`**: Reject non-pointer inputs to `Register` and `UnregisterAll` with a descriptive error instead of panicking inside `reflect.Value.Pointer()`. - **`event/servicebus.go`**: Add `objectPointer` helper that validates `reflect.Value` kind. `Register`/`Unregister`/`UnregisterAll`/`Registrations` all use it. `Registrations` now returns a copy of the internal slice so callers cannot mutate shared state without the lock. - **`util/compare.go`**: `Equal` guards against `==` on uncomparable types (slices, maps, funcs). Falls back to `reflect.DeepEqual` when either operand has a non-comparable type, preventing runtime panics. - **`typeid.go`**: Initialize `LocalTypeDomain` to a real named domain at package init instead of leaving it as a nil `TODO`. Previously `TypeIdOf` / `newIdFor` would build IDs with a nil domain. ### Larger refactors split out Two review suggestions warranted their own PRs because they touch many call sites and benefit from independent review: - **#20584**: `Publish` → `atomic.Pointer[handlerMap]` — lock-free `Publish`/`HasCallback`, copy-on-write writers. Eliminates the `RWMutex` and clone-on-publish entirely. - **#20585**: `relations` → `[]*component` — removes interface-header allocation and `unsafe.Pointer` indirection in relations storage. ## Test plan - [x] `go build ./...` - [x] `go test ./node/app/...` - [ ] CI passes 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up from #20583 — addresses the deferred "relations → []*component" suggestion from the #20450 review.
Changes the internal
relationstype from[]relation(interface slice) to[]*component. Therelationinterface is still used as the public parameter type forAdd/Remove/Contains/WithDependencies/WithDependent— callers pass anyrelationand we unwrap to*componentat the boundary via the existingasComponenthelper.Why
*component == *component, eliminating theuintptr(unsafe.Pointer(...))dance via therelationPtrhelper.*componentwas converted torelationfor storage.cmp/samePtrnow take*componentdirectly, making the intent explicit.Scope
Intentionally narrow: the
relationinterface itself is not changed, so there's no churn in method signatures that takerelationas a parameter. Only the internal storage type changes.Test plan
go test ./node/app/...🤖 Generated with Claude Code