Worst rr cruft / development taxes

Chris Jones jones.chris.g at gmail.com
Thu Jul 24 09:52:13 PDT 2014


rr developers: what parts of the code are giving you the most trouble?  Are
there particular code idioms that are confusing/awkward?  Are there
important parts of the code that are inscrutable?  Is it hard to see how
the various pieces fit together because of poor organization?

Rob has previously listed a few items that he wanted changed in the earlier
"rr refactorings" thread.  I think we've started on all but one of them:

* Introduce a TaskPtr<T>/remote<T> pointer to denote pointers that are only
meaningful in a Task's remote address space.

I have a few small-ish items on my list

* [relatively minor] migrate C idioms to C++.  rr was originally written in
C, and numerous things are more easily/clearly/safely written in C++ than
in C.  We've been migrating older C code as we go along.  For example, we
should completely eliminate manual memory management in rr.

* record_syscall.cc code that sets up "scratch" memory is rather delicate
and somewhat confusing.  Some C++ helpers to automate more of the process
would go a long way.  Even better, use syscall_defs.h macros to
auto-generate common cases.

* record_signal.cc code bends over backwards to not handle multiple pending
signals, and as a result is pretty dense.  It's probably simpler to take
the hit and implement multiple pending signals.  (Some of the dense code is
just inherent complexity, unfortunately.)

* recorder.cc code that handles restarted syscalls is quite complex and
could use a rethinking.  Linux syscall restarting is inherently
complicated, so we may not be able to trim down much.

* de-duplicate several local helpers for actions like "continue to next
syscall boundary".  Many of these that are basically-but-not-quite-the-same
was a big source of cognitive overhead for me when I started on rr.

* many syscall-hook functions in preload.c are superficially quite
similar.  Autogenerating the common cases from syscall_defs.h would be a
nice maintenance win.

* switching to the instructions-retired counter simplifies/removes a lot of
delicate perf-counter-resetting and -compensating code.  Discussed briefly
in other thread.

There's a larger architecture confusion in rr that I'd like to fix as we go
along.  rr was originally written in the C idiom of outer functions
implementing run loops around file-local state.  That state has evolved
into enclosed C++ classes, the *Session ones, but we're still driving those
sessions with the old C outer functions.  This gets particularly confusing
in replayer.cc where we have the C loops switch back and forth between
various Sessions as a result of checkpointing and gdb |call foo()|.  One
needs to be careful not to hold local variables that refer to paused
sessions, etc.

I don't have a specific plan for this yet, but the basic idea is to put
*Sessions in charge of driving themselves.  Or more specifically, the run
logic clearly acts on behalf of one and only one session.  (There are some
even bigger changes we can make once we've started along this road, but
they're so far off I don't even want to bring them up yet ;).)

Cheers,
Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/rr-dev/attachments/20140724/2a2dd22b/attachment.html>


More information about the rr-dev mailing list