Re: [PATCH v2] Optimize USDT discovery a little

Nick Alcock
2025-07-22T10:46:06Z

On 25 Jun 2025, eugene loh outgrape:

> From: Eugene Loh <eugene.loh@oracle.com>
>
> We want to reduce the performance cost of USDT discovery if possible.
>
> Specifically, a number of statements -- with probe descriptions such as
> "dtrace:::BEGIN" that could never specify USDT probes -- will not get
> their clause flags set with DT_CLSFLAG_USDT_EXCLUDE. So these statements
> get considered unnecessarily during periodic probe discovery.
>
> Therefore:
>
> *) Expand ignore_clause(dtp, n, uprp) to support the case uprp==NULL.
> This case is independent of any knowledge of a specific underlying
> probe.
>
> *) During probe discovery, check ignore_clause(dtp, i, NULL). This
> sets the DT_CLSFLAG_USDT_[INCLUDE|EXCLUDE] flag and allows faster
> exclusion of statements that do not need to be reconsidered.

I'm pretty amazed you can even detect this optimization, but the code
change feels like a good idea anyway, if just on code reuse grounds.

> To take advantage of this optimization, users should specify providers.
> E.g., instead of "BEGIN" (which could conceivably be a USDT probe),
> specify "dtrace:::BEGIN".

It's a shame this is necessary though. (In practice, nobody is ever
going to do it for BEGIN, at least -- but it's common enough with other
probes to be worth taking advantage of.)

> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

Reviewed-by: Nick Alcock <nick.alcock@oracle.com>

modulo the tiny nit below.

> - char lastchar = pdp->prv[len - 1];
> + char lastchar = (pdp->prv[0] != '\0' ? pdp->prv[len - 1] : '*');

I'd be a bit more belt-and-braces here, given that if this goes wrong
it's a buffer overrun:

> + char lastchar = ((pdp->prv[0] != '\0' && len > 0) ? pdp->prv[len - 1] : '*');

> @@ -555,6 +559,8 @@ ignore_clause(dtrace_hdl_t *dtp, int n, const dt_probe_t *uprp)
> }
> if (dt_stmt_clsflag_test(stp, DT_CLSFLAG_USDT_EXCLUDE) == 1)
> return 1;
> + if (uprp == NULL)
> + return 0;

This could do with a blank line above the first if in this hunk, I think?

>
> /*
> * If we cannot ignore this statement, try to use uprp.
> @@ -751,13 +757,9 @@ static int discover(dtrace_hdl_t *dtp)
> */
> memset(&pcb, 0, sizeof(dt_pcb_t));
> for (i = 0; i < dtp->dt_stmt_nextid; i++) {
> - dtrace_stmtdesc_t *stp;
> -
> - stp = dtp->dt_stmts[i];
> - if (stp == NULL)
> + if (ignore_clause(dtp, i, NULL))
> continue;
> - if (dt_stmt_clsflag_test(stp, DT_CLSFLAG_USDT_EXCLUDE) != 1)
> - dt_pid_create_usdt_probes(&stp->dtsd_ecbdesc->dted_probe, dtp, &pcb);
> + dt_pid_create_usdt_probes(&dtp->dt_stmts[i]->dtsd_ecbdesc->dted_probe, dtp, &pcb);

(straight moves into the callee, ok.)

--
NULL && (void)