Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allocation counter tests don't work on macOS #2672

Closed
gmilos opened this issue Mar 5, 2024 · 1 comment · Fixed by #2673
Closed

Allocation counter tests don't work on macOS #2672

gmilos opened this issue Mar 5, 2024 · 1 comment · Fixed by #2673

Comments

@gmilos
Copy link
Contributor

gmilos commented Mar 5, 2024 •
edited
Loading

Expected behavior

Documentation describes how to run allocation tests locally on macOS, here: https://github.com/apple/swift-nio/blob/63e8c4e1aa6a98252f7838b313c9e35a7317b31c/docs/debugging-allocations.md#understanding-the-output-of-the-allocation-counter-tests

The expected output should contain the actual allocation counters, like:

test_future_lots_of_callbacks.total_allocations: 75001

Actual behavior

All counters are 0, e.g.:

$ cd IntegrationTests/tests_04_performance/test_01_resources/
$ ./run-nio-alloc-counter-tests.sh test_scheduling_10000_executions.swift
...
Build complete! (0.59s)
test_scheduling_10000_executions.total_allocations: 0
test_scheduling_10000_executions.remaining_allocations: 0
test_scheduling_10000_executions.total_allocated_bytes: 0
test_scheduling_10000_executions.leaked_fds: 0

Steps to reproduce

  1. As above.

If possible, minimal yet complete reproducer code (or URL to code)

As above.

SwiftNIO version/commit hash

63e8c4e

System & version information

$ sw_vers
ProductName:		macOS
ProductVersion:		14.4
BuildVersion:		23E$X
@gmilos
Copy link
Contributor Author

gmilos commented Mar 5, 2024

Ah, the problem is that the main test package, created by ./run-nio-alloc-counter-tests.sh is mis-structured. It contains the main test package which depends on AtomicCounter and HookedFunctions:

        .package(url: "HookedFunctions/", branch: "main"),
        .package(url: "AtomicCounter/", branch: "main"),

With AtomicCounter being a static .library. HookedFunctions also depends on AtomicCounter, but it is a .dynamic library. Which means that the counters collected by HookedFunctions are separate than the counters inspected by the main test binary afterwards.

The following change appears to fix the issue[1]. However, I've only tested in on macOS. We'd need to check Linux is also working fine.

[1]

diff --git a/IntegrationTests/allocation-counter-tests-framework/template/AtomicCounter/Package.swift b/IntegrationTests/allocation-counter-tests-framework/template/AtomicCounter/Package.swift
index 3e2edbf4..c1872d9e 100644
--- a/IntegrationTests/allocation-counter-tests-framework/template/AtomicCounter/Package.swift
    b/IntegrationTests/allocation-counter-tests-framework/template/AtomicCounter/Package.swift
@@ -19,7  19,7 @@ import PackageDescription
 let package = Package(
     name: "AtomicCounter",
     products: [
-        .library(name: "AtomicCounter", targets: ["AtomicCounter"]),
         .library(name: "AtomicCounter", type: .dynamic, targets: ["AtomicCounter"]),
     ],
     dependencies: [ ],
     targets: [

gmilos added a commit to gmilos/swift-nio that referenced this issue Mar 6, 2024
@gmilos
Fix memory allocations counters on macOS.
0e271bb 
Motivation:

Allocation counters are broken on macOS as described [here](apple#2672). This PR fixes it.

Modifications:

`AtomicCounter` library is made `.dynamic`. This deduplicates the two (static) copies that used to exist. One embedded in the main binary, and the other in `HookedFunctions` dylib. This deplication fixes the issue, because there is just one copy of the counters in the process address space.

Result:

All the tests counting allocations and related statistics work on macOS.
@gmilos gmilos mentioned this issue Mar 6, 2024
Merged
Lukasa pushed a commit that referenced this issue Mar 6, 2024
@gmilos
Fix memory allocations counters on macOS. (#2673)
fc63f0c 
Motivation:

Allocation counters are broken on macOS as described [here](#2672). This PR fixes it.

Modifications:

`AtomicCounter` library is made `.dynamic`. This deduplicates the two (static) copies that used to exist. One embedded in the main binary, and the other in `HookedFunctions` dylib. This deplication fixes the issue, because there is just one copy of the counters in the process address space.

Result:

All the tests counting allocations and related statistics work on macOS.
@glbrntt glbrntt linked a pull request Apr 9, 2024 that will close this issue
Fix memory allocations counters on macOS. #2673
Merged
@glbrntt glbrntt closed this as completed Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix memory allocations counters on macOS. gmilos/swift-nio
2 participants
@glbrntt @gmilos