Repository navigation
Port MemoryCache.read() to Rust, behind an autogate - #8066
Draft
bartlomieju wants to merge 3 commits into
Draft
bartlomieju wants to merge 3 commits into
bartlomieju wants to merge 3 commits into
Conversation
A promise's reactions shared their data until the garbage collector dropped them, though a promise settles once and only one reaction can run. C++ JSG drops a ThenCatchPair as soon as one runs, so resources the data holds, such as trace spans, are released then. Reaction data now sits in a cell that the reaction that runs empties once its callback returns. then() and catch() register a pass-through for the other outcome, as C++ JSG's identityPromiseContinuation() does, so their data is dropped however the promise settles. The local_promise_then() and local_promise_catch() shims are no longer used and are removed.
Wraps v8::Value::ToDetailString(), which describes a value as C++ kj::str() of a v8::Local does, without running JavaScript such as a toString() method. The Rust MemoryCache uses it for the error tag of a failed fallback's trace span, which must match the C++ one.
Adds src/workerd/api/memory-cache-api, a Rust MemoryCache over the cache in src/rust/memory-cache, built on the io-context crate's await_io(), add_object() and trace spans. With the RUST_MEMORY_CACHE autogate enabled, memory cache bindings are the Rust MemoryCache; otherwise they stay the C++ one. MemoryCacheProvider::bindForRust() hands the Rust MemoryCache the same shared namespaces that the C++ one uses. A new test in memory-cache-test.c++ runs reads that cover every path of read() against both implementations, and checks they record the same spans. SpanRecorder now records spans in the order they open, since the order they end in depends on when each implementation releases them. New JS tests cover a fallback abandoned when its request ends, the async context a fallback runs in, the key a fallback gets for a lone surrogate, a NaN expiration, and a fallback inside a Durable Object's blockConcurrencyWhile(). They run against both implementations through the @all-autogates variant.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## biwanczuk/rust-io-context-trace-spans #8066 +/- ##
========================================================================
Coverage ? 70.04%
========================================================================
Files ? 706
Lines ? 182826
Branches ? 29286
========================================================================
Hits ? 128062
Misses ? 37635
Partials ? 17129 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Ports
MemoryCache.read()to Rust JSG, behind a newRUST_MEMORY_CACHEautogate. This is the last PR of a stack: #7963 → #8030 → #8034 → this one.Note
This branch includes a cherry-pick of #8064 (Drop Rust JSG promise reaction data once a reaction runs), which the port depends on. When #8064 lands, I'll rebase and that commit will drop out. Please review it in #8064.
Commits to review
Local::to_detail_string()to Rust JSG. Wrapsv8::Value::ToDetailString(). It describes a value the way C++kj::str()of av8::Localdoes ("TypeError: message","[object Object]", and so on) without running any JavaScript. The Rust MemoryCache uses it for a failed fallback'sfallback_errortag.MemoryCache.read()to Rust, behind an autogate.src/workerd/api/memory-cache-apiholds the RustMemoryCache. It sits on top of the cache insrc/rust/memory-cacheand uses the io-context crate'sawait_io(),add_object()and trace spans.lib.rs:read(), key validation, and cache hits.fallback.rs: a read that waits as the leader (until it runs its fallback) or as a waiter (until the read it waits for finishes).trace.rs: the same spans and tags thatmemory-cache.c++records.MemoryCacheProvider::bindForRust()gives the Rust implementation the same shared namespaces the C++ one uses.workerd-api.c++creates the RustMemoryCachefor memory cache bindings when the autogate is enabled.Behaviour
The Rust implementation is meant to be indistinguishable from the C++ one, including its trace spans. A few details:
blockConcurrencyWhile()critical section, rather than waiting on the input gate.Testing
Span parity: a new test in
memory-cache-test.c++runs the same scenario against both implementations and checks they record the same spans.SpanRecordernow records spans in the order they open, not the order they end. When each implementation releases its spans is an implementation detail.New JS tests in
memory-cache-test.js:blockConcurrencyWhile().The
@all-autogatesvariant runs all the JS tests against the Rust implementation.Known gap
Rust JSG doesn't throw DOMExceptions yet (#7951). For a value that can't be serialized, Rust throws a plain
Errorwhere C++ throws aDataCloneError, so thefallback_errortags differ. The parity scenario leaves that case out, with aTODO(#7951).