Skip to content

Replace pthreads with std::thread and friends#1859

Merged
olupton merged 32 commits into
masterfrom
olupton/drop-pthread
Jun 23, 2022
Merged

Replace pthreads with std::thread and friends#1859
olupton merged 32 commits into
masterfrom
olupton/drop-pthread

Conversation

@olupton

@olupton olupton commented Jun 21, 2022

Copy link
Copy Markdown
Collaborator

Replace pthread_* with std::thread and friends.

Also drop some old code hidden behind PERMANENT macros that was apparently never used.

Closes #1853.

@olupton olupton changed the title Olupton/drop pthread Replace pthreads with std::thread and friends Jun 21, 2022
@codecov-commenter

codecov-commenter commented Jun 21, 2022

Copy link
Copy Markdown

Codecov Report

Merging #1859 (3210255) into master (c0e2ea9) will decrease coverage by 0.04%.
The diff coverage is 57.98%.

@@            Coverage Diff             @@
##           master    #1859      +/-   ##
==========================================
- Coverage   47.17%   47.12%   -0.05%     
==========================================
  Files         543      544       +1     
  Lines      112970   112931      -39     
==========================================
- Hits        53289    53220      -69     
- Misses      59681    59711      +30     
Impacted Files Coverage Δ
src/ivoc/ivoc.cpp 49.06% <ø> (+0.23%) ⬆️
src/ivoc/ivocvect.h 95.12% <ø> (ø)
src/nrncvode/cvodeobj.cpp 85.99% <ø> (+0.01%) ⬆️
src/nrncvode/netcvode.cpp 88.31% <ø> (ø)
src/nrncvode/sptbinq.h 100.00% <ø> (ø)
src/nrniv/netpar.cpp 97.73% <ø> (+<0.01%) ⬆️
src/nrniv/nvector_nrnthread.cpp 56.03% <ø> (+0.23%) ⬆️
src/nrniv/nvector_nrnthread_ld.cpp 52.67% <ø> (+0.25%) ⬆️
src/nrnmpi/nrnmpi.cpp 73.27% <ø> (ø)
src/nrnpython/rxd.h 100.00% <ø> (ø)
... and 16 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Comment thread CMakeLists.txt Outdated
@olupton olupton force-pushed the olupton/drop-pthread branch from 5e20264 to a6962cf Compare June 21, 2022 14:45
Comment thread src/nrnoc/multicore.cpp Outdated
@azure-pipelines

Copy link
Copy Markdown

✔️ a6962cf -> Azure artifacts URL

@azure-pipelines

Copy link
Copy Markdown

✔️ 6249924 -> Azure artifacts URL

@azure-pipelines

Copy link
Copy Markdown

✔️ b84d82d -> Azure artifacts URL

@azure-pipelines

Copy link
Copy Markdown

✔️ b70b9ff -> Azure artifacts URL

@olupton olupton requested a review from nrnhines June 23, 2022 07:28
@alexsavulescu

Copy link
Copy Markdown
Member

Azure artifacts URL

https://github.com/neuronsimulator/nrn-modeldb-ci/actions/runs/2548067243

@azure-pipelines

Copy link
Copy Markdown

✔️ 3a1d8ce -> Azure artifacts URL

@azure-pipelines

Copy link
Copy Markdown

✔️ 3210255 -> Azure artifacts URL

@nrnhines

Copy link
Copy Markdown
Member

I'm trying to build on vbox guest windows 10 with 9.0.dev-34-gf8c2d4521 On branch olupton/drop-pthread

$ cmake .. -G Ninja -DCMAKE_INSTALL_PREFIX=/c/marshalnrn/nrn -DPYTHON_EXECUTABLE=/e/python39/python -DNRN_ENABLE_MPI_DYNAMIC=ON -DCMAKE_PREFIX_PATH="/c/ms-mpi" -DNRN_ENABLE_RX3D=OFF -DNRN_ENABLE_PYTHON_DYNAMIC=ON -DNRN_PYTHON_DYNAMIC="e:/python39/python"

but get

CMake Error at external/iv/CMakeLists.txt:186 (add_subdirectory):
  add_subdirectory given source "src/lib" which is not an existing directory.

I guess I can just try again...
but a little later am surprised to see

-- Looking for pthread.h
-- Looking for pthread.h - found

I guess I thought all of that would be gone.

Anyway after fixing up the iv clone with git restore --staged . ; git restore . ; git clean -d -f, cmake succeeds.
I'm on the ninja install phase now and will check terminal/gui interactivity soon.

@alexsavulescu

Copy link
Copy Markdown
Member

but a little later am surprised to see

std::thread uses pthread at a lower level

@nrnhines

Copy link
Copy Markdown
Member

I'm satisfied that terminal/gui interaction is working.

@nrnhines nrnhines left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Comment thread src/nrnoc/multicore.cpp
@azure-pipelines

Copy link
Copy Markdown

✔️ f8c2d45 -> Azure artifacts URL

@pramodk

pramodk commented Jun 23, 2022

Copy link
Copy Markdown
Member

Copying from Slack: In order to test threading functionality on windows:

I did was launch python from the bash terminal and manually

from neuron import h, gui
g = h.Graph()
# in the Graph top left menu square select View/ObjectName and see if it prints Graph[0] to the terminal.
# also select Plotwhat? and cancel the poped up symbol chooser.
g.unmap() # should close the Graph
g.view(2) # should open it again
del g # should destroy it
# NEURONMainMenu/Build/CellBuilder should open a CellBuild[0]. Just Close it
makecellbuilder() # should open a CellBuild[0]. Click ContinuousCreate and a red checkmark should appear there.
h.topology() # should print something like `|-|       soma(0-1)`
# NEURONMainMenu/File/Quit to end the session.

@nrnhines

Copy link
Copy Markdown
Member

Sometime in the distant future I'll modifiy the three nrn/share/lib/hoc/parcom.hoc tool lines that have the phrase :

sprint(nprocstr, "%d useful processors", np)

@azure-pipelines

Copy link
Copy Markdown

✔️ b04aee3 -> Azure artifacts URL

@olupton olupton merged commit bd2c7ac into master Jun 23, 2022
@olupton olupton deleted the olupton/drop-pthread branch June 23, 2022 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

C++: Replace pthreads with std::thread

6 participants