Skip to content

fix: Charge pipe I/O cycles before scheduler-side copies#5240

Closed
Officeyutong wants to merge 1 commit into
nervosnetwork:developfrom
Officeyutong:fix-script-pipe-io-cycle-accounting
Closed

fix: Charge pipe I/O cycles before scheduler-side copies#5240
Officeyutong wants to merge 1 commit into
nervosnetwork:developfrom
Officeyutong:fix-script-pipe-io-cycle-accounting

Conversation

@Officeyutong

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Issue Number: close #xxx

Problem Summary:
Pipe read/write transferred-byte cycles were added to the participating VM-local cycle counters during process_io(). If a writer stayed blocked in WaitForWrite and the root VM terminated before that writer became runnable again, those pending VM-local cycles could be discarded when non-root VMs were purged.

The host-side pipe copy also happened before the scheduler consumed and enforced the transfer cycles for the current iteration, so large pipe transfers could perform memory-copy work before the remaining cycle budget reflected that cost.

What is changed and how it works?

Proposal: xxx

What's Changed:

Charge pipe transfer cycles directly to the scheduler iteration counter in process_io() before performing the memory copy. process_io() now receives the current remaining cycle limit and returns CyclesExceeded if the transfer would exceed that budget.

The scheduler then consumes the updated iteration_cycles in the same iterate_outer() iteration, so pipe I/O cycles cannot be lost when a blocked VM is later purged. The existing exact-cycle spawn test was updated for the corrected accounting timing.

Related changes

  • PR to update owner/repo:
  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression
  • Breaking backward compatibility

@Officeyutong Officeyutong marked this pull request as ready for review June 16, 2026 11:21
@Officeyutong Officeyutong requested a review from a team as a code owner June 16, 2026 11:21
@Officeyutong Officeyutong requested review from zhangsoledad and removed request for a team June 16, 2026 11:21
@eval-exec eval-exec requested review from XuJiandong and mohanson June 16, 2026 11:26
@mohanson

Copy link
Copy Markdown
Contributor

This change affects the script's cycles statistics, which may cause network forks.

@XuJiandong XuJiandong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The cycles are part of consensus

@Officeyutong

Copy link
Copy Markdown
Collaborator Author

The cycles are part of consensus

So it's better now to make these changes?

@XuJiandong

Copy link
Copy Markdown
Contributor

It can be changed in next hard fork.

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.

3 participants