Multithreaded histogram FillRandom()

Hello dear ROOTers,
a colleague of mine asked me whether it would be possible to use multiple threads when filling histograms with a lot of data. He wants to generate some histograms to test his code on. So I came up with the following crude idea:
I use multiple histograms, one is the final histo. The rest are exactly as many as there are threads to be used. Each thread would fill it’s own histo, the main thread would wait for all of them to finish and then simply sum the thread-specific histos in the single final histo. This way, there could be no trouble by accessing the same data from multiple threads (each thread just receives it’s own histo to work on), there would be no need for mutex locks etc.

I went on to make a proof of concept code, but run into a difficulty. The code tends to crash pretty often with weird and various errors. With 4 threads, it finishes fine less than 1 in 50 runs. With more threads, the chance for fine execution quickly drops, with two, it is fine noticeably more often. Single launched worker thread never crashes. So it’s obvious the problem is with thread collisions.
However, when I introduced a sleep with variable time at the beginning of each thread, the problem went completely away.

The code (compiles without any warning): [code]#include
#include <pthread.h>
//#include <unistd.h> //SLEEP FIX

#include “TString.h”
#include “TCanvas.h”
#include “TH1D.h”

#define NUM_THREADS 4

#define NUM_FILLS 4e8

#define HISTO_RANGE 5
#define HISTO_BINS 100

using namespace std;

//volatile int sleepTime=0; //SLEEP FIX

void *threadFill(void *inHisto)
{
// sleep(sleepTime++); //SLEEP FIX
TH1D histo;
histo = (TH1D
)inHisto;

cout << Form("Fill started for histo %s\n", histo->GetTitle()) << flush;
histo->FillRandom("gaus", NUM_FILLS/NUM_THREADS);
cout << Form("Fill finished for histo %s\n", histo->GetTitle()) << flush;

pthread_exit(NULL);

}

int main()
{
TString hname = “randgaus”;
TCanvas *can = new TCanvas(hname, hname);

TH1D *h = new TH1D(hname, hname, HISTO_BINS, -HISTO_RANGE, HISTO_RANGE);

if(NUM_THREADS>0)
{
	TH1D *histosThreads[NUM_THREADS];
	
	pthread_t threads[NUM_THREADS]; //array storing threads
	int rc;
	
	for(int iThread=0; iThread<NUM_THREADS; ++iThread)
	{
		TString thrName = Form("hThread_%d", iThread);
		histosThreads[iThread] = new TH1D(thrName, thrName, HISTO_BINS, -HISTO_RANGE, HISTO_RANGE);
		
		rc = pthread_create(&threads[iThread], NULL, threadFill, (void*)histosThreads[iThread]);
		if(rc) cout << "Error:unable to create thread, rc=" << rc << endl;
	}
	
	//wait for all threads to finish
	for(int iThread=0; iThread<NUM_THREADS; ++iThread)
	{
		//waits for thread iThread to finish, if it is still running
		pthread_join(threads[iThread], NULL);
	}
	cout << "All threads finished." << endl;
	
	//adds histos to a single final histo
	for(int iThread=0; iThread<NUM_THREADS; ++iThread)
	{
		h->Add(histosThreads[iThread], 1);
		delete histosThreads[iThread];
	}
}

h->Draw();

cout << "Mean: " << h->GetMean() << endl << "RMS: " << h->GetRMS() << endl;
can->Print(hname+".gif");

pthread_exit(NULL);

}[/code]

Makefile (some flags are unnecessary, just fom my template Makefile):[code]ROOT_INCLUDE=root-config --cflags
ROOT_LIBS=root-config --libs

CC=g++
CFLAGS=-c -Wall #-Wno-unused-variable -Wno-unused-but-set-variable -Wno-sign-compare
LDFLAGS=-lMinuit -lMathCore -lMathMore -lTreePlayer -pthread
SOURCES=histoFill.cxx
OBJECTS=$(SOURCES:.cxx=.o)
EXECUTABLE=hfill
DEBUG=
OPTIM=-O3

.PHONY: all clean

all: $(SOURCES) $(EXECUTABLE)

$(EXECUTABLE): $(OBJECTS)
@echo -e “\e[1;32mLinking\e[0m”
$(CC) $(OBJECTS) $(DEBUG) $(OPTIM) -o $@ $(ROOT_LIBS) $(LDFLAGS)

%.o: %.cxx
@echo -e “\e[1;33mCompiling $@\e[0m”
$(CC) $(CFLAGS) $< $(DEBUG) $(OPTIM) -o $@ $(ROOT_INCLUDE)

clean:
rm $(OBJECTS)
[/code]

Sample fine execution output: ./hfill Fill started for histo hThread_0 Fill started for histo hThread_3 Fill started for histo hThread_1 Fill started for histo hThread_2 Fill finished for histo hThread_3 Fill finished for histo hThread_1 Fill finished for histo hThread_2 Fill finished for histo hThread_0 All threads finished. Mean: 1.2426e-05 RMS: 1.00079 Info in <TCanvas::Print>: gif file randgaus.gif has been created

Sample errors (4 different executions): [code]./hfill
hThread_2Fill started for histo hThread_3
Fill started for histo hThread_2
Fill started for histo hThread_1
Error: Missing one of ‘)’ expected at or after line 1.
Error: Unexpected end of file (G__fgetstream_newtemplate():2) (tmpfile):3:
Warning: TF1::InitStandardFunctions(} � Missing ‘;’ (tmpfile):1:
Warning: Automatic variable ) is allocated :0:
Error: Undeclared variable ) :0:
*** Interpreter error recovered ***
Error in TH1D::FillRandom: Unknown function: gaus

*** Break *** segmentation violation
*** Interpreter error recovered ***
Error in TH1D::FillRandom: Unknown function: gaus

*** Break *** segmentation violation

*** Break *** segmentation violation
Fill finished for histo hThread_1
*** Error in `./hfill’: double free or corruption (fasttop): 0x0000000001f92c40 ***
======= Backtrace: =========
(…)

./hfill
Fill started for histo hThread_0
Fill started for histo hThread_2
Fill started for histo hThread_1
Fill started for histo hThread_3
Error: Too many ‘}’ (tmpfile):2:
*** Interpreter error recovered ***
Error in TH1D::FillRandom: Unknown function: gaus
*** Interpreter error recovered ***
Error in TH1D::FillRandom: Unknown function: gaus

*** Break *** segmentation violation

*** Break *** segmentation violation

*** Break *** segmentation violation
Segmentation fault

./hfill
Fill started for histo hThread_0
Fill started for histo hThread_1
Fill started for histo hThread_3
Fill started for histo hThread_2
Warning: iStanans) Syntax error??? (tmpfile):1:
Error: Missing one of ‘)’ expected at or after line 1.
Error: Unexpected end of file (G__fgetstream_newtemplate():2) (tmpfile):3:
Error: Function InitStandardFunctions() is not defined in current scope (tmpfile):3:
Error: Missing whitespace at or after line 3.
Error: Unexpected end of file (G__fgetspace():2)Error: Symbol iStanans) is not defined in current scope (tmpfile):-1:
(tmpfile):-1:
Warning: iStanans)� Missing ‘;’ (tmpfile):-1:
*** Interpreter error recovered ***
*** Interpreter error recovered ***
*** Interpreter error recovered ***
Error in TH1D::FillRandom: Unknown function: gaus
Error in TH1D::FillRandom: Unknown function: gaus
Error in TH1D::FillRandom: Unknown function: gaus
Fill finished for histo hThread_2

*** Break *** segmentation violation

*** Break *** segmentation violation
Fill finished for histo hThread_1
*** Error in `./hfill’: double free or corruption (!prev): 0x0000000001616790 ***
======= Backtrace: =========
(…)

./hfill
Fill started for histo hThread_0
Fill started for histo hThread_2
Fill started for histo hThread_1
Fill started for histo hThread_3
Error: Missing one of ‘)’ expected at or after line 1.
Error: Unexpected end of file (G__fgetstream_newtemplate():2) (tmpfile):-1:
Error: Missing whitespace at or after line -1.
Error: Unexpected end of file (G__fgetspace():2) (tmpfile):-1:
Warning: ion(} � Missing ‘;’ (tmpfile):-1:
Warning: Automatic variable fType is allocated (tmpfile):-1:
Warning: Undeclared data member fParMax (tmpfile):1:
*** Error in `./hfill’: corrupted double-linked list: 0x00007fd2fc0008e0 ***
!!!Fatal Error: Interpreter memory overwritten by illegal access.!!!
!!!Terminate session!!!
Warning: Automatic variable fMaximum is allocated (tmpfile):-1:

*** Break *** segmentation violation
Warning: Automatic variable fMethodCall is allocated (tmpfile):-1:
Warning: Automatic variable fCintFunc is allocated (tmpfile):-1:
Warning: Illegal numerical expression Double_t (tmpfile):-1:
Warning: Illegal numerical expression Double_t (tmpfile):-1:
Warning: Illegal numerical expression Double_t (tmpfile):-1:
Warning: Illegal numerical expression Double_t (tmpfile):-1:
Warning: Illegal numerical expression Double_t (tmpfile):-1:
Error: unsigned can not be specified for float or double Double_t (tmpfile):-1:
Warning: Automatic variable TNamed is allocated (tmpfile):-1:
Warning: Automatic variable fNconst is allocated (tmpfile):-1:
Warning: Automatic variable fOptimal is allocated (tmpfile):-1:
Warning: Illegal numerical expression params (tmpfile):-1:
Error: Incorrect referencing of $Double_t (tmpfile):-1:
Warning: Automatic variable Double_t is allocated (tmpfile):-1:
Warning: Automatic variable Double_t is allocated (tmpfile):-1:
Warning: Automatic variable kGreaterThan is allocated (tmpfile):-1:
Warning: Automatic variable kasinh is allocated (tmpfile):-1:
Warning: Automatic variable minimum is allocated (tmpfile):-1:
Warning: Illegal numerical expression 81ror (tmpfile):-1:
Warning: Illegal numerical expression parmin (tmpfile):-1:
Warning: Illegal numerical expression parmax (tmpfile):-1:
Warning: Illegal numerical expression p (tmpfile):-1:
Warning: Automatic variable kxexpo is allocated (tmpfile):-1:

*** Break *** segmentation violation
!!!Fatal Error: Interpreter memory overwritten by illegal access.!!!
!!!Terminate session!!!

*** Break *** segmentation violation[/code]

To test the case with the timer fix, uncomment lines marked with //SLEEP FIX in the source code.

My question is: why is this happening and is there a better way to avoid the problem than the thread start time separation? I guess some ROOT code used by the FillRandom() method is not re-entrant or some similar thread safety issue is present. Sure, I could just run multiple processes with histo saved to rootfile, those then added. But I’d like to keep the multithread approach if possible.

Could it work when I use the ROOT’s own multithread support instead? Now I use simple POSIX threads here.

My system: openSUSE 13.1 (64bit), g++ 4.8.1, ROOT 5.34/19

Hi, I do not reply directly to your question but the thread safety has been improved in the latest version of ROOT : 6.06.

Thanks a lot for the hint!
As this was the first time I was dealing with threads in ROOT, I completely missed one crucial piece of information - that I need to explicitly turn on the multithreading support.
It works now with ROOT6 by calling ROOT::EnableThreadSafety() as well as with ROOT5 by calling TThread::Initialize() anywhere before creating the threads, eventhough I’m not using ROOT’s implementation of threads, but pthread directly.
So ROOT6 is not really needed, but the changelog you pointed me to contained all the necessary information :slight_smile:
Thanks again!
Tom