-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature - Cooperative Program Loading #33715
Feature - Cooperative Program Loading #33715
Conversation
let mut missing = Vec::new(); | ||
let mut unloaded = Vec::new(); | ||
let found = keys |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not strictly related to this PR since it was already like this, but I think this
should be a loop not a filter_map.
The iterator is filtering found
, and at the same time populating missing
, setting some stats,
and now also populating waiting_list
. Seems arbitrary and confusing that one is done through the
iterator and the rest imperatively. As a rule of thumb, I think iterators should be for one sequence
-> one sequence transformations.
4408b4b
to
92e6a61
Compare
if let Some(task) = loaded_programs_cache.next_cooperative_loading_task() { | ||
task | ||
} else { | ||
// Waiting for some other TX batch to complete loading the programs needed by this TX batch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But won't this spin?
I think we should sleep when there's no work, we could use Condvar or something to sleep until the
threads that are compiling our programs signal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, it is a spin lock. Can make it ore efficient later, still working on correctness though.
92e6a61
to
1b4d9b9
Compare
4183994
to
c036405
Compare
c036405
to
0a6d0c7
Compare
de72937
to
b0e6d78
Compare
edf1dcb
to
90c139c
Compare
…lenish(). Adds LoadedPrograms::remove_program().
…ms::cooperative_loading_task_complete().
90c139c
to
305dcd2
Compare
Closed in favor of #34407. |
Problem
Currently transaction batches race in parallel to load, verify and compile missing cache entries.
Instead they should be coordinating and split these tasks to distribute the workload across the threads and to avoid redundant work.
Summary of Changes
LoadedPrograms::replenish()
andLoadedProgramsForTxBatch::replenish()
.LoadedProgramType::Loading
andLoadedProgram::new_loading()
.LoadedPrograms::get_cooperative_loading_task()
andLoadedPrograms::submit_cooperative_loading_task()
.