Merge pull request #80 from ix-dcourtois/fix/threadsafe_interpreter

Make Interpreter's evaluations thread safe

.gitignore

@@ -8,3 +8,7 @@
 CMakeCache.txt
 src/SeExpr/parser
 src/SeExprEditor/generated
+.vs
+.vscode
+CMakeLists.txt.user
+CMakeSettings.json

src/SeExpr/Expression.cpp

@@ -306,7 +306,7 @@ const double* Expression::evalFP(VarBlock* varBlock) const {
     if (_isValid) {
         if (_evaluationStrategy == UseInterpreter) {
             _interpreter->eval(varBlock);
-            return &_interpreter->d[_returnSlot];
+            return (varBlock && varBlock->threadSafe) ? &(varBlock->d[_returnSlot]) : &_interpreter->d[_returnSlot];
         } else {  // useLLVM
             return _llvmEvaluator->evalFP(varBlock);
         }
@@ -341,7 +341,7 @@ const char* Expression::evalStr(VarBlock* varBlock) const {
     if (_isValid) {
         if (_evaluationStrategy == UseInterpreter) {
             _interpreter->eval(varBlock);
-            return _interpreter->s[_returnSlot];
+            return (varBlock && varBlock->threadSafe) ? varBlock->s[_returnSlot] : _interpreter->s[_returnSlot];
         } else {  // useLLVM
             return _llvmEvaluator->evalStr(varBlock);
         }

src/SeExpr/Interpreter.cpp

@@ -29,13 +29,30 @@
 namespace SeExpr2 {
 
 void Interpreter::eval(VarBlock* block, bool debug) {
+    // get pointers to the working data
+    double* fp = d.data();
+    char** str = s.data();
+
+    // if we have a VarBlock instance, we need to update the working data
     if (block) {
-        static_assert(sizeof(char*) == sizeof(size_t), "Expect to fit size_t in char*");
-        s[0] = reinterpret_cast<char*>(block->data());
-        s[1] = reinterpret_cast<char*>(block->indirectIndex);
+        // if the VarBlock is flagged as thread safe, copy the interpreter's data to it.
+        if (block->threadSafe == true) {
+            // copy double data
+            block->d.resize(d.size());
+            fp = block->d.data();
+            memcpy(fp, d.data(), d.size() * sizeof(double));
+
+            // copy string data
+            block->s.resize(s.size());
+            str = block->s.data();
+            memcpy(str, s.data(), s.size() * sizeof(char*));
+        }
+
+        // set the variable evaluation data
+        str[0] = reinterpret_cast<char*>(block->data());
+        str[1] = reinterpret_cast<char*>(block->indirectIndex);
     }
-    double* fp = &d[0];
-    char** str = &s[0];
+
     int pc = _pcStart;
     int end = ops.size();
     while (pc < end) {

src/SeExpr/VarBlock.h

@@ -26,22 +26,26 @@ namespace SeExpr2 {
 class ExprNode;
 class ExprVarNode;
 class ExprFunc;
-class Expression;
-class Interpreter;
 
 class VarBlockCreator;
 
 /// A thread local evaluation context. Just allocate and fill in with data.
 class VarBlock {
   private:
     /// Allocate an VarBlock
-    VarBlock(int size) : indirectIndex(0) { _dataPtrs.resize(size); }
+    VarBlock(int size, bool makeThreadSafe) : indirectIndex(0), threadSafe(makeThreadSafe), _dataPtrs(size) {}
 
   public:
     friend class VarBlockCreator;
 
     /// Move semantics is the only allowed way to change the structure
-    VarBlock(VarBlock&& other) { _dataPtrs = std::move(other._dataPtrs); }
+    VarBlock(VarBlock&& other) {
+        threadSafe = other.threadSafe;
+        d = std::move(other.d);
+        s = std::move(other.s);
+        _dataPtrs = std::move(other._dataPtrs);
+        indirectIndex = other.indirectIndex;
+    }
 
     ~VarBlock() {}
 
@@ -57,11 +61,20 @@ class VarBlock {
     // i.e.  _dataPtrs[someAttributeOffset][indirectIndex]
     int indirectIndex;
 
+    /// if true, interpreter's data will be copied to this instance before evaluation.
+    bool threadSafe;
+
+    /// copy of Interpreter's double data
+    std::vector<double> d;
+
+    /// copy of Interpreter's str data
+    std::vector<char*> s;
+
     /// Raw data of the data block pointer (used by compiler)
     char** data() { return _dataPtrs.data(); }
 
   private:
-    /// This stores double* or char** ptrs
+    /// This stores double* or char** ptrs to variables
     std::vector<char*> _dataPtrs;
 };
 
@@ -97,7 +110,17 @@ class VarBlockCreator {
     }
 
     /// Get an evaluation handle (one needed per thread)
-    VarBlock create() { return VarBlock(_nextOffset); }
+    /// \param makeThreadSafe
+    ///     If true, right before evaluating the expression, all data used
+    ///     by the interpreter will be copied to the var block, to make the
+    ///     evaluation thread safe (assuming there's one var block instead
+    ///     per thread)
+    ///     If false or not specified, the old behavior occurs (var block
+    ///     will only hold variables sources and optionally output data,
+    ///     and the interpreter will work on its internal data)
+    VarBlock create(bool makeThreadSafe = false) {
+        return VarBlock(_nextOffset, makeThreadSafe);
+    }
 
     /// Resolve the variable using anything in the data block (call from resolveVar in Expr subclass)
     ExprVarRef* resolveVar(const std::string& name) const {