Skip to content
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

Refactor drmemtrace scheduler to specialize code by mapping option #6831

Open
derekbruening opened this issue May 28, 2024 · 2 comments
Open

Comments

@derekbruening
Copy link
Contributor

Many drmemtrace scheduler routines have big conditionals dispatching on mapping options. We may be able to simplify the code if we refactor to use subclasses or templates to specialize scheduler code based on mapping options.

derekbruening added a commit that referenced this issue Nov 11, 2024
This first step toward separating key scheduler code into separate
subclasses by mode of operation adds the pImpl idiom to the scheduler:
the scheduler_t implementation is moved to a private class
scheduler_impl_t, to let us (in a future step) create the appropriate
subclass without changing callers who construct a scheduler_t.  This
also helps to split up and modularize the enormous scheduler class.

A future step will move the scheduler_impl_t code into a new file
scheduler_impl.cpp.  That was separated out to make the diff easier to
read here.

The bulk of the changes involve:
+ The name change scheduler_tmpl_t to scheduler_impl_tmpl_t.
+ Having to qualify enum types as we do not have C++20's using enum.
+ Having to refer to types in another class now that the impl class
  is separate from the outer class.
+ Moving inlined routines out of scheduler.h now that the implementation
  is private and scheduler_impl_tmpl_t is an incomplete type in scheduler.h.
+ Some sched_type_t qualifications are removed due to new (private) using
  statements to make the code more readable.

Issue: #6831
derekbruening added a commit that referenced this issue Nov 12, 2024
Moves the scheduler_impl_tmpl_t implementation out of scheduler.cpp
and into a new file scheduler_impl.cpp.

Issue: #6831
derekbruening added a commit that referenced this issue Nov 12, 2024
This first step toward separating key scheduler code into separate
subclasses by mode of operation adds the pImpl idiom to the scheduler:
the scheduler_t implementation is moved to a private class
scheduler_impl_t, to let us (in a future step) create the appropriate
subclass without changing callers who construct a scheduler_t. This also
helps to split up and modularize the enormous scheduler class.

A future step will move the scheduler_impl_t code into a new file
scheduler_impl.cpp. That was separated out to make the diff easier to
read here.

The bulk of the changes involve:
+ The name change scheduler_tmpl_t to scheduler_impl_tmpl_t.
+ Having to qualify enum types as we do not have C++20's using enum.
+ Having to refer to types in another class now that the impl class is
separate from the outer class.
+ Moving inlined routines out of scheduler.h now that the implementation
is private and scheduler_impl_tmpl_t is an incomplete type in
scheduler.h.
+ Some sched_type_t qualifications are removed due to new (private)
using statements to make the code more readable.

Issue: #6831
derekbruening added a commit that referenced this issue Nov 12, 2024
Moves the scheduler_impl_tmpl_t implementation out of scheduler.cpp and
into a new file scheduler_impl.cpp.

Issue: #6831
derekbruening added a commit that referenced this issue Nov 12, 2024
Splits the bulk of pick_next_input() into a new virtual method
pick_next_input_try() which is implemented by new subclasses
scheduler_{dynamic,fixed,replay}.cpp.
Moves pick_next_input_as_previously() into the _replay version.

Issue: #6831
derekbruening added a commit that referenced this issue Nov 13, 2024
Splits the bulk of pick_next_input() into a new virtual method
pick_next_input_for_mode() which is implemented by new subclasses
scheduler_{dynamic,fixed,replay}.cpp.
Moves pick_next_input_as_previously() into the _replay version.

Issue: #6831
derekbruening added a commit that referenced this issue Nov 13, 2024
Splits the mode-specific middle of next_record() into a new virtual method
process_next_record_candidate() which is implemented in the new subclasses
scheduler_{dynamic,fixed,replay}.cpp.

Issue: #6831
derekbruening added a commit that referenced this issue Nov 13, 2024
Adds used include files to the 3 scheduler subclasses that were
omitted when new overrides were added in step 3.

Issue: #6831
derekbruening added a commit that referenced this issue Nov 13, 2024
Adds used include files to the 3 scheduler subclasses that were omitted
when new overrides were added in step 3.

Issue: #6831
derekbruening added a commit that referenced this issue Nov 13, 2024
…ts (#7079)

Splits the mode-specific middle of next_record() into a new virtual
method check_for_input_switch() for identifying whether to trigger a
switch and call pick_next_input_for_mode(). The new method is
implemented in the new subclasses scheduler_{dynamic,fixed,replay}.cpp.

Issue: #6831
derekbruening added a commit that referenced this issue Nov 14, 2024
Splits set_initial_schedule() into separate overrides in the 3
scheduler subclasses, sharing the initial content reading in init()
prior to calling set_initial_schedule().

Splits set_output_active(): it is only valid in dynamic.

Moves rebalance_queues() and two rebalance vars to dynamic.

Moves read_recorded_schedule() and
read_and_instantiate_traced_schedule() to replay.

Adds more using statements to remove the need for "this->" prefixes on
frequent base class member field references.

Issue: #6831
@derekbruening
Copy link
Contributor Author

Though file size is not enough by itself, it does give a general, rough indication of modularity.
Before, scheduler.cpp was enormous:

$ wc ../src/clients/drcachesim/scheduler/*
   253    874   7507 ../src/clients/drcachesim/scheduler/flexible_queue.h
  4658  19106 222947 ../src/clients/drcachesim/scheduler/scheduler.cpp
  2178  11132  97930 ../src/clients/drcachesim/scheduler/scheduler.h
   133    656   4943 ../src/clients/drcachesim/scheduler/speculator.cpp
   109    507   3915 ../src/clients/drcachesim/scheduler/speculator.h

After 7 steps (2 not yet merged) it is still large but not nearly as bad:

$ wc ../src/clients/drcachesim/scheduler/*
     253     874    7507 ../src/clients/drcachesim/scheduler/flexible_queue.h
     352    1035   13057 ../src/clients/drcachesim/scheduler/scheduler.cpp
    1284    7144   59422 ../src/clients/drcachesim/scheduler/scheduler.h
    1100    4737   54957 ../src/clients/drcachesim/scheduler/scheduler_dynamic.cpp
     171     767    7798 ../src/clients/drcachesim/scheduler/scheduler_fixed.cpp
    2981   11603  133463 ../src/clients/drcachesim/scheduler/scheduler_impl.cpp
    1150    5089   50449 ../src/clients/drcachesim/scheduler/scheduler_impl.h
     521    2665   28390 ../src/clients/drcachesim/scheduler/scheduler_replay.cpp
     133     656    4943 ../src/clients/drcachesim/scheduler/speculator.cpp
     109     507    3915 ../src/clients/drcachesim/scheduler/speculator.h

derekbruening added a commit that referenced this issue Nov 14, 2024
Splits eof_or_idle() into separate overrides in the 3 scheduler
subclasses.  This then allows moving pop_from_ready_queue*() into the
dynamic subclass.

Moves process_markers(), ready_queue_empty(), and
syscall_incurs_switch() into the dynamic subclass.

Issue: #6831
@derekbruening
Copy link
Contributor Author

derekbruening commented Nov 14, 2024

The main goals of this initial round of refactoring are A) simplify the huge methods with if.else splits based on mode; B) split the code up better as scheduler.cpp was very very large. The interfaces between the main class the subclasses could be more clearly separated and crystalized; that will likely be left for a future pass.

derekbruening added a commit that referenced this issue Nov 14, 2024
Splits set_initial_schedule() into separate overrides in the 3 scheduler
subclasses, sharing the initial content reading in init() prior to
calling set_initial_schedule().

Makes set_output_active() virtual: it is only valid in dynamic.

Moves rebalance_queues() and two rebalance vars to dynamic.

Moves read_recorded_schedule() and
read_and_instantiate_traced_schedule() to replay.

Adds more using statements to remove the need for "this->" prefixes on
frequent base class member field references.

Issue: #6831
derekbruening added a commit that referenced this issue Nov 15, 2024
Splits eof_or_idle() into separate overrides in the 3 scheduler
subclasses. This then allows moving pop_from_ready_queue*() into the
dynamic subclass.

Moves process_markers(), ready_queue_empty(), and
syscall_incurs_switch() into the dynamic subclass.

Issue: #6831
derekbruening added a commit that referenced this issue Nov 19, 2024
Adds swap_in_input() and swap_out_input() subclass hooks inside
set_cur_input().  This allows moving ready queue functions into the
dynamic subclass, and will be used there for future live workload
tracking for #7067.

Moves add_to_ready_queue(), add_to_ready_queue_hold_locks(),
add_to_unscheduled_queue(), and print_queue_stats() into the dynamic
subclass.

Issue: #7067, #6831
derekbruening added a commit that referenced this issue Nov 21, 2024
Adds swap_in_input() and swap_out_input() subclass hooks inside
set_cur_input(). This allows moving ready queue functions into the
dynamic subclass, and will be used there for future live workload
tracking for #7067.

Moves add_to_ready_queue(), add_to_ready_queue_hold_locks(),
add_to_unscheduled_queue(), and print_queue_stats() into the dynamic
subclass.

Issue: #7067, #6831
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant