831104 - [meta] Adding Telemetry probe triggers mass recompilation

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P3)

Description

[Gregory Szorc [:gps]]

When I added a new Telemetry probe today, I noticed that the mere act of introducing a new probe caused recompilation of a number of C++ files. Furthermore, ccache failed to get any cache hits when recompiling these files. Presumably the probe list is being written out to a unified header and this is invalidating build system dependencies and ccache output.

This is kinda annoying. It's not stop-the-world-and-change-Telemetry-now annoying. But, it's something I wish we didn't have to live with.

I'm not sure if there is an easy low-hanging fruit solution to this.

I'd recommend organizing probes into granular "namespaces" and have each namespace written to a separate header file. That way, a change to a probe only affects direct users of the small group that probe belongs to. Unfortunately, I'm not sure if you could do this while preserving the existing Telemetry API. Perhaps if you used Java class notification for probe names then extracted the first component as a namespace/header/grouping? I dunno.

I'm not expecting any immediate action on this bug. But, if you find yourself making an invasive change to Telemetry's API, please consider the build system!

Comment 1

[Gregory Szorc [:gps]]

FWIW, the transition to moz.build files may facilitate a solution. We could expose a variable to moz.build files, let's call it "TELEMETRY_PROBE_FILES" or even "TELEMETRY_PROBES." That would allow you to define probes in multiple files. Once you've done that, you're practically at a solution!

Comment 2

[Nathan Froyd [:froydnj]]

I don't understand why it's expected that adding (or removing) a probe is necessarily a low-overhead thing.  Probe names are exposed to C++ sources, after all.

I could see an argument for segmenting the probes depending on whether you're using them from C++ or from JS; then at least adding probes for browser-specific operations or services in JS wouldn't recompile the world.  But it's not clear to me that optimizing things for adding a probe (not a particularly common event) is worthwhile.

Comment 3

[Gregory Szorc [:gps]]

(In reply to Nathan Froyd (:froydnj) from comment #2)
> I don't understand why it's expected that adding (or removing) a probe is
> necessarily a low-overhead thing.  Probe names are exposed to C++ sources,
> after all.
> 
> I could see an argument for segmenting the probes depending on whether
> you're using them from C++ or from JS; then at least adding probes for
> browser-specific operations or services in JS wouldn't recompile the world. 
> But it's not clear to me that optimizing things for adding a probe (not a
> particularly common event) is worthwhile.

I concede this is relatively low on the "reasons why the build system is slow and things we should fix" list. That being said, it is on the list. I just wanted to put the issue on your radar. If you have no intent of fixing, I'm fine with resolving WONTFIX until the day it might become a priority.

Comment 5

[Georg Fritzsche [:gfritzsche]]

This seems to be duped against the wrong bug.
Did you have another one in mind?

Comment 6

[Alessio Placitelli [:Dexter]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #5)
> This seems to be duped against the wrong bug.
> Did you have another one in mind?

Uhm, right. I can't find the other one right now, let me re-open this meanwhile.

Comment 7

[Chris H-C :chutten]

We now have decent build faster support.