Multithread fix for Antlr4 C++ target

[Alainmarcel]

Hi Mike,
I have a proposed fix that fixed all my multi-threading issues with the C++ target.
In IntervalSet.cpp, it involves guarding with a mutex all functions modifying the _intervals member.
The runtime performance hit is negligible and it prevents all crashing.
Note I'm getting a speedup that is linear with the number of threads by splitting the files to parse appropriately and scheduling the pieces evenly in between threads.


static std::mutex mutexInt;

IntervalSet::IntervalSet(int n, ...) : IntervalSet() {
  std::lock_guard<std::mutex> lg(mutexInt);
  va_list vlist;
  va_start(vlist, n);

  for (int i = 0; i < n; i++) {
    add(va_arg(vlist, int));
  }
}
......

void IntervalSet::clear() {
  std::lock_guard<std::mutex> lg(mutexInt);
  if (_readonly) {
    throw IllegalStateException("can't alter read only IntervalSet");
  }
  _intervals.clear();
}

......

[Mike Lischke]

Hi Alain,

I have a proposed fix that fixed all my multi-threading issues with the C++ target.

In IntervalSet.cpp, it involves guarding with a mutex all functions modifying the _intervals member.
The runtime performance hit is negligible and it prevents all crashing.

Have you seen this pending PR: https://github.com/antlr/antlr4/pull/1930/commits/d7f5e1834bfcee98661889cf70a5eb6824e09c1e? This contains fixes for shared access in IntervalSet (mostly the read-only flag).

Note I'm getting a speedup that is linear with the number of threads by splitting the files to parse appropriately and scheduling the pieces evenly in between threads.


static std::mutex mutexInt;

IntervalSet::IntervalSet(int n, ...) : IntervalSet() {
  std::lock_guard<std::mutex> lg(mutexInt);

This makes no sense. How can a mutex in a c-tor help with synchronization? An object that is not yet created can certainly not be used by multiple threads.

  va_list vlist;
  va_start(vlist, n);

  for (int i = 0; i < n; i++) {
    add(va_arg(vlist, int));
  }
}
......

void IntervalSet::clear() {
  std::lock_guard<std::mutex> lg(mutexInt);
  if (_readonly) {
    throw IllegalStateException("can't alter read only IntervalSet");
  }
  _intervals.clear();
}

This is what the mentioned patch addresses, but in a slightly better way (using an atomic for quick checks + such a mutex).

Mike
-- 
www.soft-gems.net

[Alainmarcel]

Hi Mike,
correct, I happily guarded everywhere without paying attention that I was also guarding the c-tor....

I'll check the latest fixes, it looks like they should address what I was seeing.

Thanks
Alain