Allow hooking Tracing.h hooks for swift-benchmark

[hassila]

Description It seems that that TaskGroup.addTask doesn't properly use the _swift_retain runtime calls.

We found this when testing https://github.com/ordo-one/package-benchmark that has hooks added to get metrics for the aggregate number of retain/release calls.

As you can see in the sample code here the number of superfluous releases matches the number of addTask calls exactly, so we assume it is not using the normal _swift_retain runtime hook that package-benchmark uses for measuring.

import Benchmark

let benchmarks = {
    func concurrentWork(tasks: Int) async {
        _ = await withTaskGroup(of: Void.self, returning: Void.self, body: { taskGroup in
            for _ in 0 ..< tasks {
                taskGroup.addTask {
                }
            }

            for await _ in taskGroup {}
        })
    }

    Benchmark("Retain/release deviation",
              configuration: .init(metrics: BenchmarkMetric.arc, maxDuration: .seconds(3))) { _ in
        await concurrentWork(tasks: 789)
    }
}

Steps to reproduce

hassila@max /tmp> gh repo clone ordo-one/external-reproducers
...
hassila@max /tmp> cd external-reproducers/swift/retain-release-diff/
hassila@max /t/e/s/retain-release-diff (main)> swift package benchmark
...
====================================================================================================
Baseline 'Current run'
====================================================================================================

Host 'max.local' with 10 'arm64' processors with 64 GB memory, running:
Darwin Kernel Version 22.3.0: Mon Jan 30 20:38:37 PST 2023; root:xnu-8792.81.3~2/RELEASE_ARM64_T6000

===================
retain-release-diff
===================

Retain/release deviation
╒════════════════════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╕
│ Metric                 │      p0 │     p25 │     p50 │     p75 │     p90 │     p99 │    p100 │ Samples │
╞════════════════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╡
│ Releases               │    2368 │    2369 │    2369 │    2369 │    2369 │    2369 │    2373 │    3425 │
├────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Retain / Release Δ     │     789 │     789 │     789 │     789 │     789 │     789 │     793 │    3425 │
├────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Retains                │    1579 │    1580 │    1580 │    1580 │    1580 │    1580 │    1580 │    3425 │
╘════════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛

Expected behavior I expect Retain / Release Δ to be zero for this test.

Environment

• Swift compiler version info swift-driver version: 1.62.15 Apple Swift version 5.7.1 (swiftlang-5.7.1.135.3 clang-1400.0.29.51) Target: arm64-apple-macosx13.0
• Xcode version info Xcode 14.1 Build version 14B47b
• Deployment target: macOS 13

[ktoso]

I added the sample code inline, thanks!

I think it may be because tasks are self owning when we create them, so there might not be explicit retain calls here. I wonder what we should do here so the result isn't suprising...

Thanks for the report! It's fantastic to have such support in the benchmark tool

[ktoso]

Hmm... for what it's worth swift_retain is the proper way as far as I can tell, and Task does the same. But we can give this a go

[ktoso]

Things work just as intended and the "imbalance" in the count is just because objects START at 1 count -- so the "1 more release than retains" is exactly correct.

You could hook the object alloc hook as well:

static HeapObject *_swift_allocObject_(HeapMetadata const *metadata,
                                       size_t requiredSize,
                                       size_t requiredAlignmentMask)
                                       asm("__swift_allocObject_");

and account for that being as-if a 1 count.

Alternatively, showing the delta count just perhaps isn't a good idea to begin with perhaps?

[hassila]

For the record, I did hook up allocObject (as well as tryRetain) and while it removed several discrepancies, it did not seem to fix this specific case...

[ktoso]

Hah you're right actually -- tasks don't hit this actually, perhaps you can try hooking

SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift)
AsyncTaskAndContext swift_task_create_common(
    size_t taskCreateFlags,
    TaskOptionRecord *options,
    const Metadata *futureResultType,
    TaskContinuationFunction *function, void *closureContext,
    size_t initialContextSize);

though you'd need to only account for creates of not async let tasks, as those are not reference counted actually.

[hassila]

Thanks, I tried to hook it in the same way as the other runtime stuff, but that gives me a SIGBUS:

Termination Reason:    Namespace SIGNAL, Code 10 Bus error: 10
Terminating Process:   exc handler [57505]

VM Region Info: 0x22b51f188 is in 0x22b4d9000-0x22b534000;  bytes after start: 287112  bytes before end: 85623
      REGION TYPE                    START - END         [ VSIZE] PRT/MAX SHRMOD  REGION DETAIL
      unused __DATA               22a767000-22b4d9000    [ 13.4M] r-x/r-x SM=COW  ...ed lib __DATA
--->  __TEXT                      22b4d9000-22b534000    [  364K] r-x/r-x SM=COW  ...urrency.dylib
      unused __DATA               22b534000-22b577000    [  268K] r-x/r-x SM=COW  ...ed lib __DATA

@ktoso suggested to hook https://github.com/apple/swift/blob/main/stdlib/public/Concurrency/Tracing.h (which was also previously discussed at the Swift Forums) - @mikeash could you possibly provide some hints on how to hook up the Concurrency/Tracing.h entry points?

[mikeash]

Tracing.h doesn't support hooks as currently designed. It's built to call os_signpost on Darwin and have no-op stubs elsewhere. I've chatted with @ktoso about how we can potentially allow hooking those.

On Darwin, you could use the compatibility overrides mechanism to do something similar. You can look at the compatibility library for an example of what that looks like: https://github.com/apple/swift/blob/be14c3358a1b9dfaafcc819ceab544aab273f8bb/stdlib/toolchain/Compatibility56/Overrides.cpp#L44. It's not very convenient, but it could be done.

[hassila]

Many thanks @mikeash for chiming in, I'll see if that can be a way forward (although we'd really like to support Linux too for tool consistency) - would be great with a hooking story for analytics tools that works on multi platforms in the future :-)

[mikeash]

Yeah, hopefully we can come up with something here. In the short term, if you feel like hacking up the Swift runtime, you can add code to the TracingStubs.h code to do whatever you want. Obviously not a good solution for real work, but it would let you experiment at least.

[ktoso]

Thanks for the hint about using the compat hooks -- I'll see about introducing a cross platform hooking mechanism here soon.

I kinda feel we can repurpose the issue for this because the title isn't right but there is actual work here.

[ktoso]

I have renamed the issue since this has become not about the group doing something wrong, but about exposing those hooks.