search

pieterderycke / Jace

Jace.NET is a calculation engine for the .NET platform.
MIT License
436 stars 94 forks source link

Engine ThreadSafety #31

Open BoasE opened 6 years ago

BoasE commented 6 years ago

Is the ThreadEngine meant to be thread safe so I can use it as a singleton accross threads?

pieterderycke commented 6 years ago

The be honest. I have never tested it to be the case. The main potential issue I see is with the caching of generated delegates. But I think it can definitely be made thread safe without a performance impact.

For which reason do you need to have it thread-safe?

BoasE commented 6 years ago

just to avoid unnecessary allocations. e.g. i could have a static engine .

aviita commented 3 years ago

Thread safety would also make the cache usage more efficient. In my current implementation I am not sharing the CalculationEngine at all between objects. I could definitely do something about this already, but the thread safety would make sense from caching point of view too. Now the MemoryCache and the ConcurrentDictionary it uses are created per CalculationEngine instance.

I have a usa case which relies quite heavily on Jace formula calculations and I can see that lots of time is spent on the DynamicCompiler.BuildFromula(). image

Here converting the cache to be a singleton/thread safe might also do the trick. It could be even configured in the JaceOptions if the cache is to be handled as singleton or not. Another option would be to use .Net MemoryCache directly and rely on it's cleanup and thread safety mechanisms. Built formulas should be thread safe already as they are just pure functions.

In the my case of which you can see the performance above I have only 32 different formulas, but 1168 instances of the engine.

aviita commented 3 years ago

I could improve my code by creating the Engine per user, so I would be ensured to have no concurrency issues, but even that approach leaves two problems.

  1. There is a performance penalty for new user sessions.
  2. There is memory overhead to even in having the cache per user.

I my case I think I could even have all the formulas cached as I have maximum of ~1900 formulas, but I think the real number is even much lower as that 32 different formulas for one use case already has almost maximum complexity. I have roughly 60 other cases for the formulas and they are much simpler mostly.

aviita commented 3 years ago

Ah, and I did notice I did invalid lookup to the .Net documentation. .Net Standard does not have the MemoryCache, so that must be why ConcurrentDictionary is used. So looking now more into the approach of making it static.

aviita commented 3 years ago

As for the thread safety in general. It would seem to me that ConstantRegistry for one would need work to be thread safe. And FunctionRegistry could potentially be even more problematic if shared. But if ommitting use of those two features it could be possible to make the engine a singleton. This is somewhat risky though. Maybe in that case there should be separate implementation of the CalculationEngine, something like ConcurrentCalculationEngine that would ommit the functionality that is not supported.