Skip to content

Simplify {session_,}result_with_timings types#1126

Merged
ryoqun merged 2 commits intoanza-xyz:masterfrom
ryoqun:simpler-result-with-timings
May 3, 2024
Merged

Simplify {session_,}result_with_timings types#1126
ryoqun merged 2 commits intoanza-xyz:masterfrom
ryoqun:simpler-result-with-timings

Conversation

@ryoqun
Copy link
Copy Markdown
Member

@ryoqun ryoqun commented Apr 30, 2024

Problem

After rethinking about simpler unified scheduler impl, it turned out some types are needlessly complex, which were introduced at solana-labs#34676.

Summary of Changes

Simplify them in preparation of chunked finished prs from the wip all-in-one pr: #1122

There should be no functional changes. and changes should be statically proven to be safe by human...

@ryoqun ryoqun requested a review from apfitzge April 30, 2024 15:17
(result, timings): &mut ResultWithTimings,
executed_task: Box<ExecutedTask>,
) {
timings.accumulate(&executed_task.result_with_timings.1);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

okay, I'm now very selective for piggybacking minor cleanups.... xD

result_with_timings
.take()
.unwrap_or_else(initialized_result_with_timings),
.send(std::mem::replace(
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

hint: this is the sole sender.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 1, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 82.2%. Comparing base (906dd29) to head (3a0fd8b).
Report is 14 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1126   +/-   ##
=======================================
  Coverage    82.2%    82.2%           
=======================================
  Files         880      880           
  Lines      235623   235617    -6     
=======================================
+ Hits       193699   193731   +32     
+ Misses      41924    41886   -38     

);
} else {
unreachable!();
match new_task_receiver.recv() {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

changed to match for later pr..

@ryoqun ryoqun merged commit f291ca4 into anza-xyz:master May 3, 2024
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