juan-g-bonilla
commented
1 year ago A possible solution is to ensure furnsh_c and unload_c are protected by a std::mutex which prevents different threads from calling them at the same time. While this does not ensure thread safety (as spice makes no guarantees of thread safety in general) it might make it safe enough for our purposes.
This might slow down parallel simulations, as every simulation has to wait for the rest to load their kernels. However, we can exploit the static/global nature of SPICE to load kernels only once, instead of for every simulation:
std::mutex SpiceInterface::kernel_manipulation_mutex;
std::map<std::string, int> SpiceInterface::kernel_reference_counter;
int SpiceInterface::loadSpiceKernel(char *kernelName, const char *dataPath)
{
std::string filepath = ...;
SpiceInterface::kernel_manipulation_mutex.lock();
SpiceInterface::kernel_reference_counter.try_emplace(filepath, 0);
if (SpiceInterface::kernel_reference_counter.at(filepath) <= 0)
{
// actually load kernel
}
SpiceInterface::kernel_reference_counter[filepath]++;
SpiceInterface::kernel_manipulation_mutex.unlock();
}
int SpiceInterface::unloadSpiceKernel(char *kernelName, const char *dataPath)
{
std::string filepath = ...;
SpiceInterface::kernel_manipulation_mutex.lock();
SpiceInterface::kernel_reference_counter.try_emplace(filepath, 0);
SpiceInterface::kernel_reference_counter[filepath]--;
if (SpiceInterface::kernel_reference_counter.at(filepath) <= 0)
{
// actually unload kernel
}
SpiceInterface::kernel_manipulation_mutex.unlock();
}The above code makes SpiceInterface keep a static counter of how many times was a kernel requested. The kernel is only loaded the first time it's needed, while it is unloaded once every simulation has called its unloadSpiceKernel. Assuming that loading/unloading only happen once, the rest of operations are O(1) and thus should not slow down the threads by having to go through the lock.
Possible break points:
- Individual simulations call
unloadSpiceKernelmore than once, which would lead to the counter de-synchronizing with the actual kernel needs and thus unloading beforehand. This will cause the simulation to fail. - The opposite, calling
loadSpiceKernelwithout callingunloadSpiceKernellead to kernels not unloading despite them not being necessary anymore. This will not cause the simulation to fail, but the kernels will not unload and might cause (mild?) memory issues.
I haven't run any test, this is just brainstorming, but I could take a stab at the implementation.
patkenneally
atharris-work
Describe the bug I ran a parallel simulation featuring multiple instances of
SimulationBaseClass, each of them featuring aspiceInterfacemodule. I was expecting all simulations to complete without hurdle, but ended up seeing themultiprocessing.imapholding the individual simulation executors come to a stall, with occasional error messages popping up in the terminal.The data kernel
de430.bspeven got corrupted in the process as itssha1sumturned out different as the one shipped with Basilisk.To reproduce Steps to reproduce the behavior:
Reset()method gets calledmultiprocessing.imapor multiprocessing.poolExpected behavior Basilisk is advertised as capable of running MC simulations but I don't see how the current architecture overcomes Spice's advertised not-thread safe nature. I would expect parallelized simulation to complete without hurdle but the execution stall as well as the kernel data corruption goes to show there is a snag.
Screenshots See https://github.com/AVSLab/basilisk/discussions/203
Desktop (please complete the following information):
Additional context The issue with Spice seems to be heavily tied to the library initialization where the data kernels are 'furnished'. It might just do the trick to spawn one instance of
SimulationBaseClassoutside of the parallel section and have aspiceInterface's Reset() function be called inside this specific instance, before the parallel processing takes place : if steps are taken to inhibit thespiceInterface'sloadSpiceData()functions inside the parallel processing, I think the issue of the simultaneous 'furnishing' of the data kernels will be alleviated.Maybe there's additional caution to be taken regarding the unloading of the data kernels