style: comments carry the measurement, not the essay around it

ext/ExperimentalAPITestExt.jl

@@ -14,10 +14,8 @@ using ExperimentalAPI:
     partition_holds
 using Test: Test, @test, @testset
 
-# The knobs below are the newest part of this package and the least settled: which of them a
-# project should turn on is a question no measurement has answered yet, and the answer will change
-# what they mean. The mark is written here, in the extension that defines them, which is also the
-# case `experimental(m; extensions = true)` exists for.
+# The newest part of the package: which of these a project should turn on is unmeasured. Marked
+# here, in the extension that defines them — the case `experimental(m; extensions = true)` is for.
 ExperimentalAPI.@experimental(
     "which gates a project should run, and therefore what these keywords should default to, is " *
         "undecided; `require_tracking` and `max_marks` may be replaced by one policy argument",
@@ -44,10 +42,8 @@ function ExperimentalAPI.test_surface(
     a = audit(m; methods)
     outputlevel ≥ 1 && show(stdout, MIME"text/plain"(), a)
     @testset "public surface of $(nameof(m))" begin
-        # Assertions that run whatever the audit found. Everything below iterates over a set of
-        # findings, so on a clean module all of it collapses to nothing and the testset would
-        # report `0 tests passed` — a green indistinguishable from the extension having failed to
-        # load, or from `m` having no public names at all. These make the pass mean something.
+        # Everything below iterates over findings, so a clean module reports `0 tests passed` — a green
+        # indistinguishable from the extension failing to load. These make the pass mean something.
         @testset "every public name has a docstring" begin
             @test isempty(setdiff(a.undocumented, skip))
             @test isempty(a.dangling)

src/audit.jl

@@ -1,14 +1,10 @@
-# The check. Everything above this file is material; this is the part that turns a marker into
-# something that can fail.
+# The check: the part that turns a marker into something that can fail.
 #
-# The name half of the audit is one set difference: public surface, minus the names with a
-# docstring, minus the names with a mark. What is left is the set of names a caller can reach and
-# nobody has said anything about — and it is exactly the set that a package cannot leave non-empty
-# once this runs in CI.
+# The name half is one set difference — public surface, minus documented, minus marked.
 #
-# The method half exists because `names(m)` cannot see a method a package contributed to somebody
-# else's generic, and for a package whose surface IS such methods — `fetch(model, quantity)` with
-# 570 of them — a clean name audit reports nothing while covering nothing.
+# The method half exists because `names(m)` cannot see a method contributed to somebody else's
+# generic. For a package whose surface IS such methods (`fetch` with 570 of them) a clean name
+# audit reports nothing while covering nothing.
 
 """
     surface(m::Module) -> Vector{Symbol}
@@ -93,22 +89,17 @@ This answers *whether prose exists*, never whether it is any good. A docstring r
 documented as far as this package is concerned.
 """
 function isdocumented(m::Module, name::Symbol)
-    # `Docs.hasdoc` is public API — `public`, and exported from `Base.Docs`. Its set-valued
-    # sibling `Docs.undocumented_names` answers the same question for a whole module at once and
-    # is what `Aqua.test_undocumented_names` is built on; the per-name form is used here because
-    # `audit` has to tell a module's own gap apart from a dependency's, and the set form reports
-    # a re-exported name's missing docstring as though it were this module's to fix.
+    # `Docs.hasdoc` is public API. The per-name form rather than `Docs.undocumented_names` because the
+    # set form reports a re-exported name's missing docstring as this module's to fix.
     return Base.Docs.hasdoc(m, name)
 end
 
 function isdocumented(m::Method)
     id = _ftype_identity(_sig_ftype(m.sig))
     owner = id === nothing ? m.module : id.mod
     name = id === nothing ? m.name : id.name
-    # Only the module that WROTE the method is asked. `Docs` files a docstring under the
-    # binding's module but in the *writing* module's table, so this is the right table — and
-    # looking in the owner's as well would let the generic's own docstring, whose key is
-    # `Tuple{Any, Any}`, account for all 570 methods anybody ever contributed to it.
+    # Only the module that WROTE the method. Looking in the owner's table too would let the generic's
+    # own docstring, keyed `Tuple{Any, Any}`, account for every method contributed to it.
     d = try
         Base.Docs.meta(m.module; autoinit=false)
     catch
@@ -138,10 +129,9 @@ function _argument_tuple(@nospecialize(sig))
     end
 end
 
-# Whether a mark says anything about `m`'s own public surface. A mark that attached to a
-# signature is asked about the generic it extends, not about whether the name happens to be bound
-# here: `using ..Upstream` leaves no binding for `fetch_value`, and reading that absence as "ours"
-# would report every contributed method as a dangling promise.
+# Whether a mark says anything about `m`'s own surface. A signature mark is asked about the
+# generic it extends: `using ..Upstream` leaves no binding, and reading that absence as "ours"
+# would report every contributed method as dangling.
 function _is_surface_claim(m::Module, mk::Mark)
     if mk.sig !== nothing
         id = _ftype_identity(_sig_ftype(mk.sig))
@@ -166,15 +156,12 @@ function _is_own(m::Module, name::Symbol)
     end
 end
 
-# `own_methods` is a scan over every public callable of every loaded module — 1916 candidates and
-# 11026 methods behind them for this package — and a suite that audits several modules pays it once
-# per audit. Its ANSWER, though, is "the methods whose defining module is `m`", and that set can
-# only change when a method is defined or deleted. Both bump the world counter: measured on 1.11.9,
-# 1.12.2 and 1.14.0-DEV, a method definition bumps it in all three.
+# A scan over every public callable of every loaded module — 1916 candidates, 11026 methods here.
+# The answer changes only when a method is defined or deleted, and a method definition bumps the
+# world counter on 1.11.9, 1.12.2 and 1.14.0-DEV alike.
 #
-# A `const` binding does NOT bump it on 1.11 (it does on 1.12 and later), which is why the key is
-# argued rather than assumed. A new `const` cannot change this answer: either it aliases something
-# whose methods belong to another module, or creating it defined a method and bumped the counter.
+# A `const` does NOT bump it on 1.11, but cannot change this answer either: it aliases something
+# whose methods belong elsewhere, or creating it defined a method.
 const _OWN_METHODS = Ref{Tuple{UInt64,Dict{Module,Vector{Method}}}}((
     typemax(UInt64), Dict{Module,Vector{Method}}()
 ))
@@ -223,10 +210,8 @@ function _own_methods(m::Module)
             mm.module === m && !(mm in seen) && (push!(seen, mm); push!(out, mm))
         end
     end
-    # The key is built ONCE per method, not once per comparison. `sort!(…; by = f)` calls `f` on
-    # both sides of every comparison, and `string(mm.sig)` is not cheap: measured on this
-    # package's own 301 methods, the sort was 0.601s while building all 301 keys was 0.039s. That
-    # one line was 80% of `audit`, which is called once per module in every surface check.
+    # Keys built once per method: `sort!(…; by = f)` calls `f` on both sides of every comparison. On
+    # 301 methods the sort was 0.601s and building all 301 keys 0.039s.
     return out[sortperm([(string(mm.name), string(mm.sig)) for mm in out])]
 end
 
@@ -459,10 +444,8 @@ function audit(m::Module; methods::Bool=true)
             n in marked || push!(unaccounted, n)
         end
     end
-    # A mark on a generic another module owns is the foreign-method form — `Base.show(io, ::T)`
-    # — and it promises nothing about THIS module's surface, so it cannot dangle here;
-    # `contributed_methods` is where it is accounted for. A mark on something of our own that is
-    # not public does dangle, whether or not it carries a signature.
+    # A mark on another module's generic promises nothing about this surface, so it cannot dangle —
+    # `contributed_methods` accounts for it. A mark on something of ours that is not public does.
     dangling = sort!(
         unique(
             mk.name for mk in all_marks if _is_surface_claim(m, mk) && !(mk.name in surf)

src/detect.jl

@@ -1,27 +1,19 @@
-# The default layer: which marked definitions a run actually entered.
+# The default layer: which marked definitions a run entered. Presence, not counts, and only for
+# definitions with a body.
 #
-# Scope: presence, not counts, and only for definitions with a body. A mark written as a name list,
-# or attached to a struct, const, module or macro, is a declaration only — nothing observes it.
+# The statement the macro puts in a marked body reads one field and writes it once: 1.03x on one
+# thread, 0.985x on eight, over 10M calls of a numeric body. A shared counter is 3.76x at eight
+# threads and loses 40% of its increments to races unless atomic.
 #
-# The one statement the macro puts in a marked body reads a single field and writes it once:
-# measured at 1.03x on one thread and 0.985x on eight, over 10M calls of a numeric body. A flag
-# written once and only read afterwards stops dirtying the cache line, which a counter (3.76x at
-# eight threads, and losing 40% of its increments to races unless atomic) does not.
-#
-# `record` reaches the same statement without changing it: opening a recording clears every
-# probe's flag, so the short-circuit fails and the write side runs on every call. The cost of
-# counting is paid only inside `record`, and the fast path is one field load either way.
+# `record` reaches the same statement without changing it: opening a recording clears every flag,
+# so the short-circuit fails and the write side runs on every call.
 
 # Padding, in Int64 slots, between one thread's counter and the next. A cache line is 64 bytes on
 # every platform this runs on; two threads sharing one would serialise on the store.
 const _COUNTER_STRIDE = 8
 
-# How many distinct backtraces one probe keeps while recording, and how many times it will look.
-# A backtrace costs microseconds, so capturing one per call would dominate any run long enough to
-# be worth recording. The paths a marked definition is reached by are few and repeat, so the
-# attempt budget is what bounds the cost: without it, a definition reached by three paths would
-# keep paying for a backtrace on every one of ten million calls, having found its third path in
-# the first microsecond.
+# How many distinct backtraces one probe keeps, and how many times it will look. A backtrace costs
+# microseconds and the paths repeat, so the attempt budget is what bounds the cost.
 const _TRACE_CAP = 64
 const _TRACE_ATTEMPTS = 256
 
@@ -62,13 +54,10 @@ end
 # The fast path, and the only thing a marked body does when nothing is recording.
 Base.getindex(p::Probe) = p.entered
 
-# The write side. Reached once per process when nothing is recording, and on every call while a
-# recording is open — which is what makes counting cost nothing outside `record`.
+# The write side: once per process when nothing is recording, every call while one is open.
 #
-# `@noinline` for two reasons, and neither is speed on this path. It keeps the marked body small,
-# so the fast path is a load and a branch over a call; and it makes the call a real frame, so the
-# backtrace taken underneath it resolves to the marked definition rather than to whatever the
-# optimiser left at that address.
+# `@noinline` keeps the marked body small, and makes the call a real frame so a backtrace taken
+# underneath resolves to the marked definition.
 @noinline function Base.setindex!(p::Probe, v::Bool)
     if _RECORDING[]
         _hit!(p)
@@ -116,10 +105,9 @@ function _resize_hits!(p::Probe, tid::Int)
     return nothing
 end
 
-# The address list only. Resolving it to names here would mean walking the debug info while the
-# sampling profiler may be in its signal handler doing the same thing, and the two take the same
-# lock: `record`'s own paths would deadlock against its own timing. `_trace_names` is called
-# once, at the end of the block, with the sampler stopped.
+# Addresses only. Resolving names here walks the debug info under the same lock the sampler takes
+# in its signal handler — paths would deadlock against timing. `_trace_names` runs at the end of
+# the block, sampler stopped.
 @noinline function _capture_trace!(p::Probe)
     bt = backtrace()
     @lock p.lock begin
@@ -197,26 +185,19 @@ probes() = reduce(vcat, (probes(m) for m in marked_modules()); init=Probe[])
 # What the macro puts in the body: one statement, a read that writes only on the first call.
 _probe(flag) = :($flag[] || ($flag[] = true))
 
-# Returns the definition with the probe spliced in, or `nothing` if this form has no body to
-# instrument.
+# The definition with the probe spliced in, or `nothing` if the form has no body.
 #
-# The `LineNumberNode` is the declaration's own, and it is load bearing rather than cosmetic. The
-# write side is a cold branch, so the optimiser is free to sink it to the end of the function;
-# without a location of its own it inherits whichever statement happens to be next, and a
-# backtrace taken inside it then resolves to that statement's inlining context instead of to the
-# marked definition. `record`'s call paths are built out of exactly that.
+# The `LineNumberNode` is load bearing: the write side is a cold branch the optimiser may sink, and
+# without its own location it inherits the next statement's — a backtrace taken inside it then
+# resolves to that statement's inlining context, which is what `record`'s paths are built from.
 function _instrument(def, flag, src::LineNumberNode)
     def isa Expr || return nothing
     if def.head === :macrocall
-        # An annotating macro — `@inline` and its neighbours — leaves the body alone, so the probe
-        # rides inside the definition it wraps and the wrapper is rebuilt around the result. Only
-        # those reach here: `_subject` marks a macrocall instrumentable exactly when the macro is
-        # in `_ANNOTATING_MACROS`, and refuses or opts out of every other one.
+        # An annotating macro leaves the body alone, so the probe rides inside and the wrapper is rebuilt
+        # around it. Only `_ANNOTATING_MACROS` reach here.
         #
-        # Returning `nothing` here instead — which is what this did — did not merely lose the
-        # observation. The flag is registered either way, so `@experimental "…" @inline f(x) = x`
-        # counted as an observable definition that no call could ever set: `entered` reported it
-        # as not entered no matter what ran, and `unverified` reported it forever.
+        # Returning `nothing` registered the flag anyway, so `@experimental "…" @inline f(x) = x` counted
+        # as observable and no call could ever set it.
         inner = _instrument(def.args[end], flag, src)
         inner === nothing && return nothing
         return Expr(:macrocall, def.args[1:(end - 1)]..., inner)
@@ -314,11 +295,9 @@ function marked_modules()
     return out
 end
 
-# Cached per world age. The walk is over every binding of every loaded module, and `record` asks
-# for it twice per block — with a large dependency tree loaded that is the most expensive thing
-# in a recording that counts a hundred calls. Keying on the world counter is exact rather than
-# approximate: a module gains a registry only by defining a `const`, and defining one advances
-# the counter.
+# Cached per world age: the walk is over every binding of every loaded module and `record` asks
+# twice per block. Exact, not approximate — a module gains a registry only by defining a `const`,
+# which advances the counter.
 const _MARKED_MODULES = Ref{Tuple{UInt64,Vector{Module}}}((typemax(UInt64), Module[]))
 
 function _walk_modules!(out::Vector{Module}, seen::Set{Module}, m::Module)

src/macros.jl

@@ -1,9 +1,6 @@
-# The expression-level spelling of the observing layer.
-#
-# `record(() -> f(x))` is the function form and it is what everything here is built on. The macro
-# earns its place by knowing two things a closure cannot: the source text of the expression, and
-# the line it was written on. A report that says which call went through unvalidated code, and
-# where that call is, is a different thing from a list of names.
+# The expression-level spelling of the observing layer, built on `record(() -> f(x))`. The macro
+# earns its place by knowing two things a closure cannot: the source text of the expression and
+# the line it was written on.
 
 @experimental """
 the report is a text format with no schema, and it has already changed twice in its first week — \