Worst rr cruft / development taxes

Robert O'Callahan robert at ocallahan.org
Thu Jul 24 16:32:36 PDT 2014


I agree with the items in your list.

I swear I had my own extensive list written down somewhere but I can't find
it anywhere so recreating it, or at least the parts that you didn't already
cover.

On Fri, Jul 25, 2014 at 4:52 AM, Chris Jones <jones.chris.g at gmail.com>
wrote:

> [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.
>

There are lots of C structs that should be converted to classes, with
functionality moved into members.


> 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 ;).)
>

Right.

I would like to see most of the recording and replay logic move into
RecordSession and ReplaySession (and ExperimentSession (which maybe we
could call DivergingSession?)). There are more applications for rr than
just gdb and I think the Session classes should be the basis for an "rr
API". All the gdb-specific logic should be separated from replay logic and
implemented as a control loop driving a ReplaySession.

Additional small structural changes:
* Move all perfctr management to Task. Currently a few reset_hpc/stop_hpc
calls occur from outside Task but it's cleaner to eliminate all knowledge
of hpc from outside Task.

Various cosmetic changes:
* Replace tabs with spaces.
* Reformat all code using clang-format and keep it that way. Possibly
switch to 4-space indent.
* Rename non-adjectival classes (which is almost all of them) to CamelCase
(per style guide, but currently mostly not followed).
* Within reason, have a .h and .cc file per C++ class and give the file the
same name as the class.
* Move everything into an "rr" namespace (partially started).
* Whether we move to instructions-retired counter or not, rename most usage
of "rbc/rcb" etc to something more generic like "perfcount". Fixing the
horrid inconsistency of rbc/rcb is my main priority :-).
* I know you like the pattern where a private member foo is accessed via a
getter method "foo_type& foo()" but I don't :-). It makes it impossible to
later introduce separate code paths for setting and getting foo. I'd prefer
"const foo_type& foo() const" or (just "foo_type foo()")and "void
set_foo(const foo_type&)".
* Whether or not we do that, we need a style pattern for naming private
variables so they don't clash with foo() getters. Right now it's ad-hoc.
* I'd like to nail down rules for ordering #include files.

Rob
-- 
oIo otoeololo oyooouo otohoaoto oaonoyooonoeo owohooo oioso oaonogoroyo
owoiotoho oao oboroootohoeoro oooro osoiosotoeoro owoiololo oboeo
osouobojoeocoto otooo ojouodogomoeonoto.o oAogoaoiono,o oaonoyooonoeo
owohooo
osoaoyoso otooo oao oboroootohoeoro oooro osoiosotoeoro,o o‘oRoaocoao,o’o
oioso
oaonosowoeoroaoboloeo otooo otohoeo ocooouoroto.o oAonodo oaonoyooonoeo
owohooo
osoaoyoso,o o‘oYooouo ofooooolo!o’o owoiololo oboeo oiono odoaonogoeoro
ooofo
otohoeo ofoioroeo ooofo ohoeololo.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/rr-dev/attachments/20140725/3aaeb054/attachment.html>


More information about the rr-dev mailing list