Re: [PATCH v2] Optimize USDT discovery a little

[Nick Alcock <nick.alcock@oracle.com>]

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)