[Bug tapsets/17242] New: tapsets should avoid delayed-init globals

[Bug tapsets/17242] New: tapsets should avoid delayed-init globals

[jistone at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17242

            Bug ID: 17242
           Summary: tapsets should avoid delayed-init globals
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: tapsets
          Assignee: systemtap at sourceware dot org
          Reporter: jistone at redhat dot com

We have a few tapsets which want to use some internal global data, but avoid
setting that up until they are actually called.  I believe this is a case of
harmful premature optimization.

First, we treat probe handlers as an atomic unit for locking purposes. 
Initializing global variables requires an exclusive write lock, and since we
can't predict whether that will be needed, we end up with a write lock every
time.  This is harmful to multithreaded performance when it could otherwise use
a shared read lock, having only written once the entire time.

Second, if a function ever writes a global variable, it's no longer pure, so we
can't optimize it out.  It's probably uncommon that someone would call these
functions without caring about the result, but we should still try to be clean.

The particular case which triggered this report is with _reg_offsets[] and
_stp_regs_registers, for almost every $arch/registers.stp.  These are not
expensive tables to prepare -- they should just use an early begin probe to
init and be done with it.  For scripts which don't query register info, this
still won't be pulled in at all.  Also, globals which are only written in
begin/end are even optimized not to need runtime locking at all.

We should look for other similar cases to clean up throughout the tapsets.


Tangent: lookup tables like this would also be better with a fixed size, so
they don't waste a lot of empty nodes up to MAXMAPENTRIES.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug tapsets/17242] tapsets should avoid delayed-init globals

[jistone at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17242

Josh Stone <jistone at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|systemtap at sourceware dot org    |jistone at redhat dot com

--- Comment #1 from Josh Stone <jistone at redhat dot com> ---
PR6466's commit ba6f838d2471 reveals one hiccup in this idea.  If the init+read
function isn't referenced, but something else pulls that tapset file in, then
the global array would be created, but then it would be noticed as unreferenced
and get elided.  Now with a begin probe also getting pulled in to initialize
the global, that's an independent reference to keep it around.

I think that's an acceptable tradeoff, but if we can detect and remove the
only-written global too, we'll get the best of both worlds.

-- 
You are receiving this mail because:
You are the assignee for the bug.