Skip to content

perf(amber): reuse spec ActorSystem in CheckpointSpec#4502

Closed
Yicong-Huang wants to merge 1 commit into
apache:mainfrom
Yicong-Huang:perf/checkpoint-spec-runtime
Closed

perf(amber): reuse spec ActorSystem in CheckpointSpec#4502
Yicong-Huang wants to merge 1 commit into
apache:mainfrom
Yicong-Huang:perf/checkpoint-spec-runtime

Conversation

@Yicong-Huang
Copy link
Copy Markdown
Contributor

@Yicong-Huang Yicong-Huang commented Apr 26, 2026

What changes were proposed in this PR?

CheckpointSpec was the slowest spec in CI (~70s for 2 trivial assertions) because it booted two Pekko ActorSystems:

  • one in beforeAll (CheckpointSpec.scala:62)
  • a second one created lazily inside AmberRuntime.serde when chkpt.save(...) was first called

The two systems never interacted — the second only existed so SerializationExtension had a system to attach to.

This PR adds an actorSystem_= setter on AmberRuntime and uses it in CheckpointSpec.beforeAll to hand the spec's own system to the runtime, so the lazy fallback never fires.

Any related issues, documentation, discussions?

Closes #4501

How was this PR tested?

Existing CheckpointSpec tests cover the two assertions. Verified locally that the spec still completes the same two assertions; relying on CI to confirm full timing improvement.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 4.7)

CheckpointSpec was the slowest spec in CI (~70s) because it booted two
ActorSystems: one in beforeAll and a second one lazily inside
AmberRuntime.serde when chkpt.save was first called. The two systems
never interacted — the second was only created so SerializationExtension
had a system to attach to.

Add an actorSystem setter on AmberRuntime so the spec can hand its own
system to the runtime; the lazy fallback no longer fires. Also drop
long-stale commented-out tests in CheckpointSpec.

Closes apache#4501
@Yicong-Huang Yicong-Huang force-pushed the perf/checkpoint-spec-runtime branch from df7b2ed to d494180 Compare April 26, 2026 00:12
@Yicong-Huang
Copy link
Copy Markdown
Contributor Author

Closing — analysis methodology was wrong. ScalaTest emits the [info] SpecName: header at suite end, not start, so the 70s I attributed to CheckpointSpec was actually the duration of the next spec (TrivialControlSpec). CheckpointSpec is already fast; this PR's change eliminates a real-but-trivial double-boot in serde, with negligible CI impact (320s vs 322s, within noise). Real bottleneck is TrivialControlSpec (~70s).

@Yicong-Huang Yicong-Huang deleted the perf/checkpoint-spec-runtime branch April 26, 2026 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Optimize CheckpointSpec test runtime

1 participant