JeffersonLab / sim-recon

Simulation and Reconstruction for GlueX
9 stars 14 forks source link

- remove all of the mutex locks around hddm input/output. The hddm #1087

Closed rjones30 closed 6 years ago

rjones30 commented 6 years ago

library is thread-safe already. Please do not cripple applications with locks around hddm i/o calls -- in only slows down the app and effectively disables the multi-threading capability of hddm. This fix makes hddm run 4 times faster with 4 threads, according to my benchmark test. Dunno how those mutex locks got there.

sdobbs commented 6 years ago

Richard, Just wanted to point out that this commit includes a lot of changes besides removing the mutex. Did you want to merge them now as well?

rjones30 commented 6 years ago

Sean,

Ok, yes. Let me push the rest of the updates for the hits shadowing, before you look further at it.

-Richard

On Tue, Mar 20, 2018 at 12:24 PM, Sean Dobbs notifications@github.com wrote:

Richard, Just wanted to point out that this commit includes a lot of changes besides removing the mutex. Did you want to merge them now as well?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/JeffersonLab/sim-recon/pull/1087#issuecomment-374662233, or mute the thread https://github.com/notifications/unsubscribe-auth/AHeFWMU8H8pilwCxDPPIpEnZs7kxogv_ks5tgS05gaJpZM4SyNCZ .

rjones30 commented 6 years ago

Sean,

Ok, all set. This update also includes the new hits shadowing code. Those commits have their own commit comments, so I think pulling them altogether is fine, even though that was not what I had originally intended.

-Richard

On Tue, Mar 20, 2018 at 12:29 PM, Richard Jones rjones30@gmail.com wrote:

Sean,

Ok, yes. Let me push the rest of the updates for the hits shadowing, before you look further at it.

-Richard

On Tue, Mar 20, 2018 at 12:24 PM, Sean Dobbs notifications@github.com wrote:

Richard, Just wanted to point out that this commit includes a lot of changes besides removing the mutex. Did you want to merge them now as well?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/JeffersonLab/sim-recon/pull/1087#issuecomment-374662233, or mute the thread https://github.com/notifications/unsubscribe-auth/AHeFWMU8H8pilwCxDPPIpEnZs7kxogv_ks5tgS05gaJpZM4SyNCZ .

gluex commented 6 years ago

Test status for this pull request: SUCCESS

Summary: /work/halld/pull_request_test/sim-recon^remove_locks_around_hddm_io_for_full_speed_mcsmear_in_multithread_mode_rtj/tests/summary.txt Logs: /work/halld/pull_request_test/sim-recon^remove_locks_around_hddm_io_for_full_speed_mcsmear_in_multithread_mode_rtj/tests/log

Build log: /work/halld/pull_request_test/sim-recon^remove_locks_around_hddm_io_for_full_speed_mcsmear_in_multithread_mode_rtj/make_remove_locks_around_hddm_io_for_full_speed_mcsmear_in_multithread_mode_rtj.log Build report: /work/halld/pull_request_test/sim-recon^remove_locks_around_hddm_io_for_full_speed_mcsmear_in_multithread_mode_rtj/report_remove_locks_around_hddm_io_for_full_speed_mcsmear_in_multithread_mode_rtj.txt Location of build: /work/halld/pull_request_test/sim-recon^remove_locks_around_hddm_io_for_full_speed_mcsmear_in_multithread_mode_rtj

sdobbs commented 6 years ago

OK, sounds good - I just wanted to make sure that this was what you intended, since it was originally different than the request commentary =)