Project

General

Profile

Task #2122

Stop using where()

Added by Mark Abraham almost 3 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Low
Assignee:
Category:
core library
Target version:
Difficulty:
simple
Close

Description

This debugging aid causes several of branches per MD step, and dozens of them on global-communication steps. Even if they were implemented in a no-cost way, we should be using debuggers, not printf.

Associated revisions

Revision c70211cf (diff)
Added by Mark Abraham over 2 years ago

Use gmx::Mutex and gmx::lock_guard to fix some issues

Refactored a few uses of raw thread-MPI mutexes to fix known leaks, so
valgrind and LeakSanitizer become more useful.

Discovered that the thread-MPI trylock behaviour has always been buggy
on Windows, and cannot be fixed so that the return code is consistent
across the two current implementations in the case where the calling
thread might already own the lock. Fortunately, we don't use trylocks
anywhere, and the problematic case is unlikely to occur, but the
limitation is now documented.

Added unit tests for whichever mutex implementation we are using, now
and in the future. These cover the above case.

Found bugs in _where, which presumably wasn't an issue if it only
ran when handling errors. But it runs multiple times per MD step.

Refs #2122
Refs #2123

Change-Id: If1b2f092c569f47d67ce39e6decb56f04cea314a

Revision c8c0605d (diff)
Added by Mark Abraham over 1 year ago

Remove use of where(), if DEBUG etc.

The where() debugging function causes run-time branches in
release-mode builds, and replaces functionality for which one should
use a debugger.

Other such preprocessing clutter should also go away. Since we don't
check whether any of it compiles or produces useful data, we may as
well delete it.

This coincidentally simplifies the call sigatures of some
functionality, too.

Retained some useful essentialdynamics debug code, which is now always
compiled, and written from setup code only at debug level 2.

Refs #2423
Fixes #2122

Change-Id: I2c60c162734f4c2ec1d56153853a36d28e6a66ff

History

#1 Updated by Mark Abraham almost 3 years ago

Similarly, dump_it_all should go, though at least that one only runs in debug build.

#2 Updated by Gerrit Code Review Bot almost 3 years ago

Gerrit received a related patchset '1' for Issue #2122.
Uploader: Mark Abraham ()
Change-Id: gromacs~master~If1b2f092c569f47d67ce39e6decb56f04cea314a
Gerrit URL: https://gerrit.gromacs.org/6474

#3 Updated by Mark Abraham over 2 years ago

  • Target version changed from 2018 to 2019

#4 Updated by Erik Lindahl almost 2 years ago

  • Tracker changed from Bug to Task
  • Priority changed from Normal to Low
  • Affected version - extra info deleted (all of them)
  • Affected version deleted (git master)

#5 Updated by Gerrit Code Review Bot over 1 year ago

Gerrit received a related patchset '1' for Issue #2122.
Uploader: Mark Abraham ()
Change-Id: gromacs~master~I2c60c162734f4c2ec1d56153853a36d28e6a66ff
Gerrit URL: https://gerrit.gromacs.org/7746

#6 Updated by Mark Abraham over 1 year ago

  • Status changed from New to Fix uploaded

#7 Updated by Mark Abraham over 1 year ago

  • Status changed from Fix uploaded to Resolved

#8 Updated by Mark Abraham over 1 year ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF