feat(cdp): run the rust hogvm synchronously and remove the shadow comparison - #70070
Merged
Conversation
…ison executeSync on the JS thread matches the Node VM's threading model; the libuv thread-hop of the async path cost more than the sub-millisecond executions it offloaded. Batching many events per FFI crossing is the follow-up if that changes. The shadow comparison (RustVmShadow, CDP_HOG_RUST_VM_SHADOW_SAMPLE_RATE, executeBatch + rayon) served its purpose validating parity and is removed now that the rust vm is the primary executor.
2 tasks
…hogvm-executesync # Conflicts: # nodejs/src/cdp/hog-transformations/rust-vm-executor.test.ts # nodejs/src/cdp/hog-transformations/rust-vm-executor.ts
jose-sequeira
marked this pull request as ready for review
July 13, 2026 08:11
Contributor
|
Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/mas..." | Re-trigger Greptile |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
Contributor
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
pl
approved these changes
Jul 13, 2026
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
2 participants
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.
Problem
Follow-up to #69605:
executeBatchentry point. The batch shape is only worth it when we parallelize multiple events per call; for one event at a time the libuv thread-hop isn't worth it, andexecuteSyncmatches the Node VM's synchronous exec while blocking the loop 6.7x less (168µs vs 1,127µs mean for the geoip template).Changes
RustVmExecutorexecutes throughexecuteSyncon the calling thread;execute()is synchronous again.RustVmShadow, its tests, theCDP_HOG_RUST_VM_SHADOW_SAMPLE_RATEconfig, the transformer's capture/flush hooks, and the napiexecuteBatchentry point plus its rayon machinery. Charts config removal: PostHog/charts#13069.Benchmark: geoip template, release addon, Apple Silicon
execHog(previous prod path)executeSync(this PR)executeBatch(1)awaitedexecuteBatch(1), 8 in flightexecuteBatch(128)per callHow did you test this code?
I'm an agent; checks I ran:
cargo test --features noop(19 pass, including the thread-local log carryover test for the back-to-back executeSync shape),cargo clippy -D warnings,cargo shear(rayon removal leaves no unused deps), releasecargo build.rust-vm-executor.test.ts(9 pass) reverted to the sync contract; transformer wiring tests updated to mockexecuteSync.tsc -bclean.Automatic notifications
Docs update
Internal executor change, no user-facing behavior difference.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)