Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

shared WASM execution cache #579

Merged
merged 5 commits into from
Feb 5, 2025
Merged

shared WASM execution cache #579

merged 5 commits into from
Feb 5, 2025

Conversation

sduchesneau
Copy link
Contributor

adds a cache for module*block executed near the HEAD of the chain, to prevent executing the same foundational modules 5 times whenever a block comes in

@sduchesneau sduchesneau requested a review from maoueh February 5, 2025 16:45
@@ -67,6 +77,7 @@ type Tier1Config struct {
SubrequestsEndpoint string
SubrequestsInsecure bool
SubrequestsPlaintext bool
SharedCacheSize uint64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Size in what? Bytes, count, etc?

@@ -37,6 +37,9 @@ var Tier2RejectedRequestCounter = MetricSet.NewCounterVec(
"Counter for total Substreams requests the tier2 rejected, by reason (gRPC code reason)",
)

var ExecutedWasmModules = MetricSet.NewCounter("substreams_executed_wasm_modules", "Counter for total Substreams modules x block for which wasm was executed")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get the ... x block for part

callEntries map[string]map[string]*callEntry
}

const maxUint64 uint64 = 1<<64 - 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not useful, math.MaxUint64 works out of the box


func NewSharedCache(size uint64) *SharedCache {
return &SharedCache{
headBlock: atomic.NewUint64(maxUint64),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
headBlock: atomic.NewUint64(maxUint64),
headBlock: atomic.NewUint64(math.MaxUint64),

sync.Mutex
headBlock *atomic.Uint64
size uint64
// module_hash -> blockHash -> callEntry
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need the "kind" of size, I think it's in block.

if tracer.Enabled() {
zlog.Debug("executing wasm call", zap.String("module_hash", moduleHash), zap.Uint64("block_num", clock.Number))
}
metrics.ExecutedWasmModules.Inc()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best would be to link that to the app context, so at least it would to be ever blocking

if !ok {
result = &callEntry{clock: clock}
s.callEntries[clock.Id][moduleHash] = result
resultLockOwner = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to add a comment here to explain the overall process that this is going to block other modules that arrives after this one and that it's a write lock because it's going to actually write the results.

}

s.Unlock()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be better to put that in the else clause above, tied it more closely where it matters.

argValues map[string][]byte,
) error {
clock := call.Clock
var resultLockOwner bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var resultLockOwner bool
// Manually unlocked, pay attention if you add early exit beyond that lock call

return
}
exec.GlobalSharedCache = sharedCache
hubSrc.Run()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check for errors here, at least for logging purposes?

@sduchesneau sduchesneau merged commit 9f18c58 into develop Feb 5, 2025
5 checks passed
@sduchesneau sduchesneau deleted the stepd/sharedbuffer branch February 5, 2025 21:51
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