open-simulation-platform / libcosimc

OSP C co-simulation API
Mozilla Public License 2.0
12 stars 2 forks source link

Adjusting libcosimc to fiberless libcosim #34

Closed ljamt closed 2 years ago

ljamt commented 2 years ago

Adapting to the libcosim api change.

markaren commented 2 years ago

We can probably remove fibers in libcosimc altogether?

ljamt commented 2 years ago

We can probably remove fibers in libcosimc altogether?

Yes, but then we'll need to rewrite cosim_execution_start to spin off a thread for instance..

markaren commented 2 years ago
auto task = std::thread ([execution]() {
                execution->simulate_result.set_value(execution->cpp_execution->simulate_until(std::nullopt));
            });
            execution->t = std::move(task);

where simulate_result is an std::promise might do the trick. I wanted to test, but the dlls are not placed correctly so I can't run the code and I don't want to debug this. execution_async_health_check needs to go though. Otherwise just keep this code for now.

restenb commented 2 years ago

If the fiber is only used to spawn the main thread, which now runs most of it's work on separate hardware threads, I'm not sure I see the immediate need to remove fibers here as well, at least if it would demand further changes in libcosim.

The rationale for removing fibers from libcosim was basically that they weren't being used, and just added extra weight, wasn't it? The usage here seems OK to me.

markaren commented 2 years ago

If std::promise can do the same job, we don't need the extra dependency.

ljamt commented 2 years ago

If std::promise can do the same job, we don't need the extra dependency.

I agree, but suggest to solve that as a separate issue.