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

Need restart function for scheduler to avoid recursively call scheduler->run() #478

Open
littlewin-wang opened this issue Mar 7, 2024 · 5 comments
Labels
component: sparta Issue is related to sparta framework enhancement Enhancement or request

Comments

@littlewin-wang
Copy link
Contributor

littlewin-wang commented Mar 7, 2024

In my simulator based on sparta,I need flush all inflight events in my flush process, which means I need stop, clear events and re-run the scheduler.
While my flush function is in one event handler, so actually I use scheduler->run() in recursive.

The flush event handler in my flush manager

      void FlushManager::ForwardingFlushSignal(const FlushingCriteria& flush_criteria) {
        auto tick = scheduler_->getCurrentTick();
        scheduler_->stopRunning();
        scheduler_->restartAt(tick);
        flush_criteria_ = flush_criteria;
        flush_event_.schedule(1);
        scheduler_->run();
    }

It is handler bind to one inPort

rob_flush_manager_in.registerConsumerHandler(
                CREATE_SPARTA_HANDLER_WITH_DATA(FlushManager, ForwardingFlushSignal, FlushingCriteria));

I don't want to recursively call func, it may cause memory usage problem

@klingaard
Copy link
Member

That's definitely a hard-core way to flush the events! A possible better approach might be to add a custom API to the Scheduler that allows you to flush all events at a given tick and later. It'd look like this:

      void FlushManager::ForwardingFlushSignal(const FlushingCriteria& flush_criteria) {
        auto tick = scheduler_->getCurrentTick();
        scheduler_->flushAll(tick);
        ....
    }

@littlewin-wang
Copy link
Contributor Author

I guess the clearEvents() has already fulfilled the goal of flushAll

But it needs to stop running first, and there is no method to restart running againg
I have fixed this temporarily as below

    void FlushManager::ForwardingFlushSignal(const FlushingCriteria& flush_criteria) {
        auto tick = scheduler_->getCurrentTick();
        scheduler_->stopRunning();
        scheduler_->restartAt(tick);
        flush_criteria_ = flush_criteria;
        nop_event_.schedule(0);
        flush_event_.schedule(1);
        scheduler_->keepRunning();
    }

While I add a method for scheduler

    void keepRunning() {
        running_ = true;
    }

Please note that a zero cycle nop_event must be scheduled to avoid finished judgment in scheduler->run()

And do you have a plan for the flushAll mentioned above, I'm not very familiar with the code in scheduler. 😪

@klingaard
Copy link
Member

I forgot I added clearEvents some time ago... forgot the use case.

Let me attempt to understand your use case:

  1. You've detected a flushing situation in your model and you want to flush all pending events from point "now" to the future
  2. The point in which you start the flush you've coalesced simulation, meaning you know all events after the flushing event are good for flushing

Like I mentioned in my previous post, this is quite a large hammer to flush pending work in the simulator. Typically, the flush manager receives the flush (for the next cycle) and broadcasts that flush (via ports) to all the units that need to be notified. Each unit makes the decision on what needs to be flushed from the scheduler (using cancel and cancelIf function calls in the events/ports).

@klingaard klingaard added enhancement Enhancement or request component: sparta Issue is related to sparta framework labels Mar 15, 2024
@YouqiXia
Copy link

I forgot I added clearEvents some time ago... forgot the use case.

Let me attempt to understand your use case:

  1. You've detected a flushing situation in your model and you want to flush all pending events from point "now" to the future
  2. The point in which you start the flush you've coalesced simulation, meaning you know all events after the flushing event are good for flushing

Like I mentioned in my previous post, this is quite a large hammer to flush pending work in the simulator. Typically, the flush manager receives the flush (for the next cycle) and broadcasts that flush (via ports) to all the units that need to be notified. Each unit makes the decision on what needs to be flushed from the scheduler (using cancel and cancelIf function calls in the events/ports).

We try to stop and restart the scheduler by using stopRunning() and restartAt(tick). After performing these operations and inserting some events into the scheduler, it cannot restart and run the expected events because the running_ flag is not set to true. Did we use the restartAt() function correctly? Or does the restartAt() function have the responsibility to set running_ to true?

@klingaard
Copy link
Member

I'm going through old issues and noticed I missed your response...

I'd expect the flow of scheduler use (in your use case) would be:

  • stopRunning
  • restartAt(tick)
  • run

You are correct that restartAt will not set running. After all, you're not running since you called stopRunning. Scheduler advancement (i.e. running) will only occur in the call to run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: sparta Issue is related to sparta framework enhancement Enhancement or request
Projects
None yet
Development

No branches or pull requests

3 participants