-
Notifications
You must be signed in to change notification settings - Fork 180
Add time CLI utility to supported benchmark tools
#317
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,51 @@ | ||||||
| name: Time (CLI utility) Example | ||||||
|
|
||||||
| on: | ||||||
| push: | ||||||
| branches: | ||||||
| - master | ||||||
| pull_request: | ||||||
| branches: | ||||||
| - master | ||||||
|
|
||||||
| permissions: | ||||||
| contents: write | ||||||
| deployments: write | ||||||
|
|
||||||
| jobs: | ||||||
| benchmark: | ||||||
| name: Run Some silly benchmark under `time` | ||||||
| runs-on: ubuntu-latest | ||||||
| steps: | ||||||
| - uses: actions/checkout@v3 | ||||||
| - name: Run some silly benchmark with time | ||||||
| run: (time (head --bytes=1000000000 /dev/random >/dev/null)) 2>&1 | tee output.txt | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use
🔧 Proposed fix- run: (time (head --bytes=1000000000 /dev/random >/dev/null)) 2>&1 | tee output.txt
+ run: (time (head --bytes=1000000000 /dev/urandom >/dev/null)) 2>&1 | tee output.txt📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| - name: Store benchmark result | ||||||
| uses: benchmark-action/github-action-benchmark@v1 | ||||||
| with: | ||||||
| name: Time Benchmark | ||||||
| tool: 'time' | ||||||
| output-file-path: output.txt | ||||||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||||||
| auto-push: true | ||||||
| # Show alert with commit comment on detecting possible performance regression | ||||||
| alert-threshold: '200%' | ||||||
| comment-on-alert: true | ||||||
| fail-on-alert: true | ||||||
| alert-comment-cc-users: '@ktrz','@henrikingo' | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Invalid YAML syntax for The value 🔧 Proposed fix- alert-comment-cc-users: '@ktrz','@henrikingo'
+ alert-comment-cc-users: '@ktrz,`@henrikingo`'Or as an explicit YAML list if the action supports it: alert-comment-cc-users: |
`@ktrz`
`@henrikingo`📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| - name: Store benchmark result - separate results repo | ||||||
| uses: benchmark-action/github-action-benchmark@v1 | ||||||
| with: | ||||||
| name: Time Benchmark | ||||||
| tool: 'time' | ||||||
| output-file-path: output.txt | ||||||
| github-token: ${{ secrets.BENCHMARK_ACTION_BOT_TOKEN }} | ||||||
| auto-push: true | ||||||
| # Show alert with commit comment on detecting possible performance regression | ||||||
| alert-threshold: '200%' | ||||||
| comment-on-alert: true | ||||||
| fail-on-alert: true | ||||||
| alert-comment-cc-users: '@ktrz','@henrikingo' | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same YAML syntax issue as Line 36. 🔧 Proposed fix- alert-comment-cc-users: '@ktrz','@henrikingo'
+ alert-comment-cc-users: '@ktrz,`@henrikingo`'📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| gh-repository: 'github.com/benchmark-action/github-action-benchmark-results' | ||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -26,6 +26,7 @@ This action currently supports the following tools: | |||||
| - [Benchmark.Net][benchmarkdotnet] for .Net projects | ||||||
| - [benchmarkluau](https://github.com/Roblox/luau/tree/master/bench) for Luau projects | ||||||
| - [JMH][jmh] for Java projects | ||||||
| - [time][time-cli-util] CLI utility to measure runtime time of any Linux executable | ||||||
| - Custom benchmarks where either 'biggerIsBetter' or 'smallerIsBetter' | ||||||
|
|
||||||
| Multiple languages in the same repository are supported for polyglot projects. | ||||||
|
|
@@ -51,6 +52,7 @@ definitions are in [.github/workflows/](./.github/workflows) directory. Live wor | |||||
| | .Net | [![C# Benchmark.Net Example Workflow][benchmarkdotnet-badge]][benchmarkdotnet-workflow-example] | [examples/benchmarkdotnet](./examples/benchmarkdotnet) | | ||||||
| | Java | [![Java Example Workflow][java-badge]][java-workflow-example] | [examples/java](./examples/java) | | ||||||
| | Luau | Coming soon | Coming soon | | ||||||
| | Time (CLI utility) | [`time`][time-workflow-example] | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Malformed table row - missing column and trailing pipe. The table row for "Time (CLI utility)" is missing the third column (Example Project) and the trailing pipe character, which breaks the table structure. 📝 Proposed fix-| Time (CLI utility) | [`time`][time-workflow-example]
+| Time (CLI utility) | [`time`][time-workflow-example] | [examples/time](./examples/time) |📝 Committable suggestion
Suggested change
🧰 Tools🪛 markdownlint-cli2 (0.20.0)[warning] 55-55: Table pipe style (MD055, table-pipe-style) [warning] 55-55: Table column count (MD056, table-column-count) [warning] 55-55: Table column style (MD060, table-column-style) 🤖 Prompt for AI Agents |
||||||
|
|
||||||
| All benchmark charts from above workflows are gathered in GitHub pages: | ||||||
|
|
||||||
|
|
@@ -651,6 +653,7 @@ Every release will appear on your GitHub notifications page. | |||||
| [examples-page]: https://benchmark-action.github.io/github-action-benchmark/dev/bench/ | ||||||
| [pytest-benchmark]: https://pypi.org/project/pytest-benchmark/ | ||||||
| [pytest]: https://pypi.org/project/pytest/ | ||||||
| [time-cli-util]: https://man7.org/linux/man-pages/man1/time.1.html | ||||||
| [alert-comment-example]: https://github.com/benchmark-action/github-action-benchmark/commit/077dde1c236baba9244caad4d9e82ea8399dae20#commitcomment-36047186 | ||||||
| [rust-workflow-example]: https://github.com/benchmark-action/github-action-benchmark/actions?query=workflow%3A%22Rust+Example%22 | ||||||
| [go-workflow-example]: https://github.com/benchmark-action/github-action-benchmark/actions?query=workflow%3A%22Go+Example%22 | ||||||
|
|
@@ -660,6 +663,7 @@ Every release will appear on your GitHub notifications page. | |||||
| [catch2-workflow-example]: https://github.com/benchmark-action/github-action-benchmark/actions?query=workflow%3A%22Catch2+C%2B%2B+Example%22 | ||||||
| [julia-workflow-example]: https://github.com/benchmark-action/github-action-benchmark/actions?query=workflow%3A%22Julia+Example+with+BenchmarkTools.jl%22 | ||||||
| [java-workflow-example]: https://github.com/benchmark-action/github-action-benchmark/actions?query=workflow%3A%22JMH+Example%22 | ||||||
| [time-workflow-example]: https://github.com/benchmark-action/github-action-benchmark/actions?query=workflow%3A%22time+Example%22 | ||||||
| [help-watch-release]: https://docs.github.com/en/github/receiving-notifications-about-activity-on-github/watching-and-unwatching-releases-for-a-repository | ||||||
| [help-github-token]: https://docs.github.com/en/actions/security-guides/automatic-token-authentication | ||||||
| [minimal-workflow-example]: https://github.com/benchmark-action/github-action-benchmark/actions?query=workflow%3A%22Example+for+minimal+setup%22 | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -11,7 +11,7 @@ inputs: | |||||
| required: true | ||||||
| default: 'Benchmark' | ||||||
| tool: | ||||||
| description: 'Tool to use get benchmark output. One of "cargo", "go", "benchmarkjs", "pytest", "googlecpp", "catch2", "julia", "benchmarkdotnet", "customBiggerIsBetter", "customSmallerIsBetter"' | ||||||
| description: 'Tool to use get benchmark output. One of "cargo", "go", "benchmarkjs", "pytest", "googlecpp", "catch2", "julia", "benchmarkdotnet", "time", "customBiggerIsBetter", "customSmallerIsBetter"' | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tool description is incomplete. The description is missing 📝 Proposed fix- description: 'Tool to use get benchmark output. One of "cargo", "go", "benchmarkjs", "pytest", "googlecpp", "catch2", "julia", "benchmarkdotnet", "time", "customBiggerIsBetter", "customSmallerIsBetter"'
+ description: 'Tool to use get benchmark output. One of "cargo", "go", "benchmarkjs", "pytest", "googlecpp", "catch2", "julia", "jmh", "benchmarkdotnet", "benchmarkluau", "time", "customBiggerIsBetter", "customSmallerIsBetter"'📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| required: true | ||||||
| output-file-path: | ||||||
| description: 'A path to file which contains the benchmark output' | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,41 @@ | ||||||||||
| Example for parsing output of the `time` CLI utility | ||||||||||
| ==================================================== | ||||||||||
|
|
||||||||||
| - [Workflow for this example](../../.github/workflows/time.yml) | ||||||||||
| - [Action log of this example](https://github.com/benchmark-action/github-action-benchmark/actions?query=workflow%3A%22Rust+Example%22) | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect workflow link - copy-paste error from Rust example. The action log link points to the "Rust Example" workflow instead of the "time Example" workflow. 📝 Proposed fix-- [Action log of this example](https://github.com/benchmark-action/github-action-benchmark/actions?query=workflow%3A%22Rust+Example%22)
+- [Action log of this example](https://github.com/benchmark-action/github-action-benchmark/actions?query=workflow%3A%22time+Example%22)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
| - [Benchmark results on GitHub pages](https://benchmark-action.github.io/github-action-benchmark/dev/bench/) | ||||||||||
|
|
||||||||||
| This directory shows how to use [`github-action-benchmark`](https://github.com/benchmark-action/github-action-benchmark) | ||||||||||
| with [`cargo bench`](https://doc.rust-lang.org/cargo/commands/cargo-bench.html). | ||||||||||
|
Comment on lines
+8
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect description - mentions This appears to be a copy-paste error from the Rust example. The text should describe the 📝 Proposed fix This directory shows how to use [`github-action-benchmark`](https://github.com/benchmark-action/github-action-benchmark)
-with [`cargo bench`](https://doc.rust-lang.org/cargo/commands/cargo-bench.html).
+with the [`time`](https://man7.org/linux/man-pages/man1/time.1.html) CLI utility.📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
|
|
||||||||||
| ## Run benchmarks | ||||||||||
|
|
||||||||||
| Official documentation for usage of `time`: | ||||||||||
|
|
||||||||||
| https://man7.org/linux/man-pages/man1/time.1.html | ||||||||||
|
|
||||||||||
| So to run a benchmark, simply call time first: | ||||||||||
|
|
||||||||||
| ```yaml | ||||||||||
| - name: Measure execution time with `time` | ||||||||||
| run: (time (head --bytes=1000000000 /dev/random >/dev/null)) 2>&1 | tee output.txt | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| Note the `2>&1` too capture also stderr output. By default that's where `time` will direct its output. | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo: "too" should be "to". 📝 Proposed fix-Note the `2>&1` too capture also stderr output. By default that's where `time` will direct its output.
+Note the `2>&1` to capture also stderr output. By default that's where `time` will direct its output.📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
|
|
||||||||||
|
|
||||||||||
| ## Process benchmark results | ||||||||||
|
|
||||||||||
| Store the benchmark results with step using the action. Please set `cargo` to `tool` input. | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect tool reference - says The text incorrectly instructs users to set 📝 Proposed fix-Store the benchmark results with step using the action. Please set `cargo` to `tool` input.
+Store the benchmark results with step using the action. Please set `time` to `tool` input.📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
|
|
||||||||||
| ```yaml | ||||||||||
| - name: Store benchmark result | ||||||||||
| uses: benchmark-action/github-action-benchmark@v1 | ||||||||||
| with: | ||||||||||
| tool: 'time' | ||||||||||
| output-file-path: output.txt | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| Please read ['How to use' section](https://github.com/benchmark-action/github-action-benchmark#how-to-use) for common usage. | ||||||||||
|
|
||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |||||||||||||||||||||||||
| import { promises as fs } from 'fs'; | ||||||||||||||||||||||||||
| import * as github from '@actions/github'; | ||||||||||||||||||||||||||
| import { Config, ToolType } from './config'; | ||||||||||||||||||||||||||
| import * as core from '@actions/core'; | ||||||||||||||||||||||||||
| import { z } from 'zod'; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export const BenchmarkResult = z.object({ | ||||||||||||||||||||||||||
|
|
@@ -34,7 +35,7 @@ | |||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| interface PullRequest { | ||||||||||||||||||||||||||
| [key: string]: any; | ||||||||||||||||||||||||||
| number: number; | ||||||||||||||||||||||||||
| html_url?: string; | ||||||||||||||||||||||||||
| body?: string; | ||||||||||||||||||||||||||
|
|
@@ -441,7 +442,7 @@ | |||||||||||||||||||||||||
| const extra = `mean: ${mean} ${meanUnit}\nrounds: ${stats.rounds}`; | ||||||||||||||||||||||||||
| return { name, value, unit, range, extra }; | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
| } catch (err: any) { | ||||||||||||||||||||||||||
| throw new Error( | ||||||||||||||||||||||||||
| `Output file for 'pytest' must be JSON file generated by --benchmark-json option: ${err.message}`, | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
|
@@ -452,7 +453,7 @@ | |||||||||||||||||||||||||
| let json: GoogleCppBenchmarkJson; | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| json = JSON.parse(output); | ||||||||||||||||||||||||||
| } catch (err: any) { | ||||||||||||||||||||||||||
| throw new Error( | ||||||||||||||||||||||||||
| `Output file for 'googlecpp' must be JSON file generated by --benchmark_format=json option: ${err.message}`, | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
|
@@ -580,7 +581,7 @@ | |||||||||||||||||||||||||
| return ret; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function extractJuliaBenchmarkHelper([_, bench]: JuliaBenchmarkGroup, labels: string[] = []): BenchmarkResult[] { | ||||||||||||||||||||||||||
| const res: BenchmarkResult[] = []; | ||||||||||||||||||||||||||
| for (const key in bench.data) { | ||||||||||||||||||||||||||
| const value = bench.data[key]; | ||||||||||||||||||||||||||
|
|
@@ -611,7 +612,7 @@ | |||||||||||||||||||||||||
| let json: JuliaBenchmarkJson; | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| json = JSON.parse(output); | ||||||||||||||||||||||||||
| } catch (err: any) { | ||||||||||||||||||||||||||
| throw new Error( | ||||||||||||||||||||||||||
| `Output file for 'julia' must be JSON file generated by BenchmarkTools.save("output.json", suit::BenchmarkGroup) : ${err.message}`, | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
|
@@ -629,7 +630,7 @@ | |||||||||||||||||||||||||
| let json: JmhBenchmarkJson[]; | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| json = JSON.parse(output); | ||||||||||||||||||||||||||
| } catch (err: any) { | ||||||||||||||||||||||||||
| throw new Error(`Output file for 'jmh' must be JSON file generated by -rf json option: ${err.message}`); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| return json.map((b) => { | ||||||||||||||||||||||||||
|
|
@@ -646,7 +647,7 @@ | |||||||||||||||||||||||||
| let json: BenchmarkDotNetBenchmarkJson; | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| json = JSON.parse(output); | ||||||||||||||||||||||||||
| } catch (err: any) { | ||||||||||||||||||||||||||
| throw new Error( | ||||||||||||||||||||||||||
| `Output file for 'benchmarkdotnet' must be JSON file generated by '--exporters json' option or by adding the JsonExporter to your run config: ${err.message}`, | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
|
@@ -692,6 +693,93 @@ | |||||||||||||||||||||||||
| return results; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function parseTimeOutput(line: string): number | undefined { | ||||||||||||||||||||||||||
| core.debug(`parse time ${line}`); | ||||||||||||||||||||||||||
| const t = line.split('\t')[1]; | ||||||||||||||||||||||||||
| if (t === undefined) return; | ||||||||||||||||||||||||||
| const tparts = t.split('m'); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| core.debug(tparts[0]); | ||||||||||||||||||||||||||
| if (tparts[1] === undefined || !tparts[1].endsWith('s')) return; | ||||||||||||||||||||||||||
| return parseFloat(tparts[0]) * 60.0 + parseFloat(tparts[1].substring(-1)); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+696
to
+705
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical bug:
Use 🐛 Proposed fix function parseTimeOutput(line: string): number | undefined {
core.debug(`parse time ${line}`);
const t = line.split('\t')[1];
if (t === undefined) return;
const tparts = t.split('m');
core.debug(tparts[0]);
if (tparts[1] === undefined || !tparts[1].endsWith('s')) return;
- return parseFloat(tparts[0]) * 60.0 + parseFloat(tparts[1].substring(-1));
+ return parseFloat(tparts[0]) * 60.0 + parseFloat(tparts[1].slice(0, -1));
}🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| /* Expect to process text with the following structure: | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| $ time echo Hello there | ||||||||||||||||||||||||||
| Hello there | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| real 0m0,100s | ||||||||||||||||||||||||||
| user 0m0,020s | ||||||||||||||||||||||||||
| sys 0m0,003s | ||||||||||||||||||||||||||
| $ time echo Hello there | ||||||||||||||||||||||||||
| Hello again | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| real 0m0,100s | ||||||||||||||||||||||||||
| user 0m0,020s | ||||||||||||||||||||||||||
| sys 0m0,003s | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Into: | ||||||||||||||||||||||||||
| [{"name": "real", | ||||||||||||||||||||||||||
| "value": 0,1, | ||||||||||||||||||||||||||
| "unit": "s", | ||||||||||||||||||||||||||
| "extra": "testName: Hello there"}, | ||||||||||||||||||||||||||
| ... | ||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
| function extractTimeBenchmarkResult(output: string): BenchmarkResult[] { | ||||||||||||||||||||||||||
| const lines = output.split(/\n/); | ||||||||||||||||||||||||||
| const results: BenchmarkResult[] = []; | ||||||||||||||||||||||||||
| let firstline = true; | ||||||||||||||||||||||||||
| let name: string | undefined = undefined; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| for (const line of lines) { | ||||||||||||||||||||||||||
| core.debug(line); | ||||||||||||||||||||||||||
| if (firstline) { | ||||||||||||||||||||||||||
| name = line; | ||||||||||||||||||||||||||
| firstline = false; | ||||||||||||||||||||||||||
| core.debug(`firstline: ${name}`); | ||||||||||||||||||||||||||
| continue; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| if (line.startsWith('real')) { | ||||||||||||||||||||||||||
| const v = parseTimeOutput(line); | ||||||||||||||||||||||||||
| core.debug(`v: ${v}`); | ||||||||||||||||||||||||||
| if (v !== undefined) | ||||||||||||||||||||||||||
| results.push({ | ||||||||||||||||||||||||||
| extra: name, | ||||||||||||||||||||||||||
| name: 'real', | ||||||||||||||||||||||||||
| value: v, | ||||||||||||||||||||||||||
| unit: 's', | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
|
Comment on lines
+746
to
+751
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial All three benchmark entries will have the same The current implementation assigns ♻️ Proposed improvement to include command context in name if (line.startsWith('real')) {
const v = parseTimeOutput(line);
core.debug(`v: ${v}`);
if (v !== undefined)
results.push({
extra: name,
- name: 'real',
+ name: name ? `${name} - real` : 'real',
value: v,
unit: 's',
});
}Apply similar changes for 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| if (line.startsWith('user')) { | ||||||||||||||||||||||||||
| const v = parseTimeOutput(line); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| core.debug(`v: ${v}`); | ||||||||||||||||||||||||||
| if (v !== undefined) | ||||||||||||||||||||||||||
| results.push({ | ||||||||||||||||||||||||||
| extra: name, | ||||||||||||||||||||||||||
| name: 'user', | ||||||||||||||||||||||||||
| value: v, | ||||||||||||||||||||||||||
| unit: 's', | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| if (line.startsWith('sys')) { | ||||||||||||||||||||||||||
| const v = parseTimeOutput(line); | ||||||||||||||||||||||||||
| core.debug(`v: ${v}`); | ||||||||||||||||||||||||||
| if (v !== undefined) | ||||||||||||||||||||||||||
| results.push({ | ||||||||||||||||||||||||||
| extra: name, | ||||||||||||||||||||||||||
| name: 'sys', | ||||||||||||||||||||||||||
| value: v, | ||||||||||||||||||||||||||
| unit: 's', | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
| firstline = true; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| core.debug('loop'); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return results; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+728
to
+781
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider handling locale-specific decimal separators. The example in the comment block (lines 711-713) shows time output with commas as decimal separators ( 🔧 Proposed fix to handle both comma and period decimals function parseTimeOutput(line: string): number | undefined {
core.debug(`parse time ${line}`);
const t = line.split('\t')[1];
if (t === undefined) return;
const tparts = t.split('m');
core.debug(tparts[0]);
if (tparts[1] === undefined || !tparts[1].endsWith('s')) return;
- return parseFloat(tparts[0]) * 60.0 + parseFloat(tparts[1].slice(0, -1));
+ // Handle both '.' and ',' as decimal separators for locale compatibility
+ const minutes = parseFloat(tparts[0].replace(',', '.'));
+ const seconds = parseFloat(tparts[1].slice(0, -1).replace(',', '.'));
+ return minutes * 60.0 + seconds;
}🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export async function extractResult(config: Config): Promise<Benchmark> { | ||||||||||||||||||||||||||
| const output = await fs.readFile(config.outputFilePath, 'utf8'); | ||||||||||||||||||||||||||
| const { tool, githubToken, ref } = config; | ||||||||||||||||||||||||||
|
|
@@ -725,6 +813,9 @@ | |||||||||||||||||||||||||
| case 'benchmarkdotnet': | ||||||||||||||||||||||||||
| benches = extractBenchmarkDotnetResult(output); | ||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||
| case 'time': | ||||||||||||||||||||||||||
| benches = extractTimeBenchmarkResult(output); | ||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||
| case 'customBiggerIsBetter': | ||||||||||||||||||||||||||
| benches = extractCustomBenchmarkResult(output); | ||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| Hello there | ||
|
|
||
| real 0m0.100s | ||
| user 0m0.020s | ||
| sys 0m0.003s | ||
| Hello again | ||
|
|
||
| real 0m0.100s | ||
| user 0m0.020s | ||
| sys 0m0.003s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider updating to
actions/checkout@v4.v3 is still functional but v4 is the current recommended version with Node.js 20 support and performance improvements.
🔧 Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents