[none]: profiling experiment - #129
tayloraswift wants to merge 3 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces Linux perf event control to the benchmarking suite, allowing for more precise hardware/software event recording by wrapping the prctl system call. The changes include a new Perf utility and updates to the benchmark script to use perf record. Feedback focuses on ensuring cross-platform compatibility by adding #if os(Linux) guards to prevent linker errors on non-Linux systems, correcting a file path in the benchmark script, and removing debug print statements to maintain clean output.
| @_silgen_name("prctl") | ||
| private func prctl( | ||
| _ option: Int32, | ||
| _ arg2: UInt, | ||
| _ arg3: UInt, | ||
| _ arg4: UInt, | ||
| _ arg5: UInt | ||
| ) -> Int32 |
There was a problem hiding this comment.
The prctl system call is specific to Linux. Declaring it with @_silgen_name without platform guards will lead to linker errors on other platforms (like macOS) that this package supports. Wrap the declaration in #if os(Linux).
| @_silgen_name("prctl") | |
| private func prctl( | |
| _ option: Int32, | |
| _ arg2: UInt, | |
| _ arg3: UInt, | |
| _ arg4: UInt, | |
| _ arg5: UInt | |
| ) -> Int32 | |
| #if os(Linux) | |
| @_silgen_name("prctl") | |
| private func prctl( | |
| _ option: Int32, | |
| _ arg2: UInt, | |
| _ arg3: UInt, | |
| _ arg4: UInt, | |
| _ arg5: UInt | |
| ) -> Int32 | |
| #endif |
| Benchmarks/.build/release/benchmark "Benchmarks/${subject}" > ${subject}.duration | ||
|
|
||
| which perf | ||
| perf record -F 8000 -g -- Benchmarks/.build/release/benchmark Swift.symbols.json |
There was a problem hiding this comment.
The path to the JSON file in the perf record command is incorrect. Based on the previous lines in the script, the file is located in the Benchmarks/ directory.
| perf record -F 8000 -g -- Benchmarks/.build/release/benchmark Swift.symbols.json | |
| perf record -F 8000 -g -- Benchmarks/.build/release/benchmark "Benchmarks/${subject}" |
| public static func enable() { | ||
| let status: Int32 = prctl(PR_TASK_PERF_EVENTS_ENABLE, 0, 0, 0, 0) | ||
| if status != 0 { | ||
| print("⚠️ WARNING: prctl(ENABLE) failed!") | ||
| } else { | ||
| print("enabled") | ||
| } | ||
| } |
There was a problem hiding this comment.
The enable() method should be platform-guarded to prevent calling prctl on non-Linux systems. Also, removing the print("enabled") message is recommended to avoid polluting the benchmark's standard output.
| public static func enable() { | |
| let status: Int32 = prctl(PR_TASK_PERF_EVENTS_ENABLE, 0, 0, 0, 0) | |
| if status != 0 { | |
| print("⚠️ WARNING: prctl(ENABLE) failed!") | |
| } else { | |
| print("enabled") | |
| } | |
| } | |
| public static func enable() { | |
| #if os(Linux) | |
| let status: Int32 = prctl(PR_TASK_PERF_EVENTS_ENABLE, 0, 0, 0, 0) | |
| if status != 0 { | |
| print("⚠️ WARNING: prctl(ENABLE) failed!") | |
| } | |
| #endif | |
| } |
| public static func disable() { | ||
| let status: Int32 = prctl(PR_TASK_PERF_EVENTS_DISABLE, 0, 0, 0, 0) | ||
| if status != 0 { | ||
| print("⚠️ WARNING: prctl(DISABLE) failed!") | ||
| } else { | ||
| print("disabled") | ||
| } | ||
| } |
There was a problem hiding this comment.
The disable() method should be platform-guarded. Removing the success message will also make the benchmark output cleaner.
| public static func disable() { | |
| let status: Int32 = prctl(PR_TASK_PERF_EVENTS_DISABLE, 0, 0, 0, 0) | |
| if status != 0 { | |
| print("⚠️ WARNING: prctl(DISABLE) failed!") | |
| } else { | |
| print("disabled") | |
| } | |
| } | |
| public static func disable() { | |
| #if os(Linux) | |
| let status: Int32 = prctl(PR_TASK_PERF_EVENTS_DISABLE, 0, 0, 0, 0) | |
| if status != 0 { | |
| print("⚠️ WARNING: prctl(DISABLE) failed!") | |
| } | |
| #endif | |
| } |
|
1 participant
No description provided.