[DTrace-devel] [PATCH] Action clear() should clear only one aggregation

[Eugene Loh]

On 8/14/24 11:57, Kris Van Hees wrote:

> On Tue, Aug 13, 2024 at 10:59:25PM -0400, eugene.loh at oracle.com wrote:
>> From: Eugene Loh <eugene.loh at oracle.com>
>>
>> FIXME:  Is libdtrace/libdtrace.ver correctly updated?

Incidentally, what has to be done in this file?

> I think that keeping dtrace_aggregate_walk() as-is would be better, along with
> adding a function dtrace_aggregate_clear_one(dtp, aid) that performs the
> lookup from aid to dtrae_aggdata_t *agd, and then calls
> dt__aggregate_clear_one(agd, dtp) to do the actual clearing.  There is no need
> to go through a walker function when we already know we need to operate on
> just one aggregation.

I'm not convinced.

"Just one aggregation"?  Yes, just one aggid, but possibly many keys.  
To narrow down to which agd we want, we need to know the key -- and 
clear() doesn't give us the key, plus there will generally be more than 
one key anyhow.

Now, one can argue that in the current implementation and its current 
hash function, the aggid tells us at least which hash bucket to search.  
So we could write a special, reduced loop.  Maybe so, but that would be 
wed to this particular choice of hash function, and I'd argue the 
current choice is a bad one.  E.g., consider:

         tick-10 { @pc[uregs[R_PC]] = count() }
         syscall::write:entry { @writeentry = count() }

Lots of agd's will be sharing one bucket and another agd is sitting all 
by itself.  We ought to change the hash function, and then we'd have to 
undo the "search just one bucket" optimization.

PS  Here is an oddity:

     # /usr/sbin/dtrace -n 'BEGIN { @a[1] = count(); @a[2] = count(); 
clear(@a[1]); exit(0); }'
     dtrace: description 'BEGIN ' matched 1 probe
      CPU     ID                    FUNCTION:NAME
       34      1                           :BEGIN

             1                0
             2                0

You *can* specify the key in the clear()!  But dtrace ignores the key.  
Silently.  Same behavior in Solaris.