sparcians / map

Modeling Architectural Platform
Apache License 2.0
168 stars 60 forks source link

Add CLS option to control infinite loop timeout #538

Closed furuame closed 3 weeks ago

klingaard commented 1 month ago

Interesting addition. Curious on the use case to increase the timeout since I believe it defaults to a minute. I've never seen the timeout fire on simulations I've made. However, if you're integrating your simulator with another framework, I suggest disabling the thread instead of increasing the timeout.

Regardless, allowing control of the timeout period seems reasonable, but does it have to be a Sparta command line option?

furuame commented 1 month ago

Interesting addition. Curious on the use case to increase the timeout since I believe it defaults to a minute. I've never seen the timeout fire on simulations I've made. However, if you're integrating your simulator with another framework, I suggest disabling the thread instead of increasing the timeout.

Yes exactly that scenario, basically there are two simulators in my scenario 🙃 One of them executed for a while and not moves forward. I checked and that works fine but just needs time. However, the other one thought it didn't make forward progress, so it broke. I still like to keep the SleeperThread just in case, it is still useful to find real bugs.

Regardless, allowing control of the timeout period seems reasonable, but does it have to be a Sparta command line option?

Since SleeperThread is a static class object, my currently deployed workaround is: simply call sparta::SleeperThread::getInstance()->setInfLoopSleepInterval at simulator initialization. My initial thought is: I should pass the parameter from my simulator to CLS, and let CLS handles it. What do you think?

klingaard commented 3 weeks ago

Since SleeperThread is a static class object, my currently deployed workaround is: simply call sparta::SleeperThread::getInstance()->setInfLoopSleepInterval at simulator initialization. My initial thought is: I should pass the parameter from my simulator to CLS, and let CLS handles it. What do you think?

I think it's fine, I just see it as very rare that anyone would use it. But no bother to me if you want to merge this.