Benchmark for Core.RemoveObjectFromList() implementation - #499
Conversation
``` dotnet run -f net8.0 -c Release -- all ```
Ensures the NetOffice Benchmarks project can be built. We are not running benchmarks are take too long to finish.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0820de55e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <Project Sdk="Microsoft.NET.Sdk"> | ||
| <PropertyGroup> | ||
| <OutputType>Exe</OutputType> | ||
| <TargetFrameworks>net48;net8.0;net10.0</TargetFrameworks> |
There was a problem hiding this comment.
The benchmark project builds net8.0 and net10.0, but its only ProjectReference points to Source/NetOffice/NetOffice.csproj, which targets only net462. Those target graphs cannot resolve a compatible NetOffice asset, so the dotnet build command in .github/workflows/benchmarks.yml fails instead of validating the benchmarks; either restrict this project to a compatible .NET Framework target or make the referenced library available for the modern targets.
Useful? React with 👍 / 👎.
|
|
||
| Run all benchmarks: | ||
| ```bash | ||
| cd Tests/NetOffice.Benchmarks |
There was a problem hiding this comment.
Because this project declares multiple target frameworks, dotnet run -c Release cannot choose an executable and exits with NETSDK1129 asking for --framework. This affects the default command and all three argument variants shown below it, so users following the README cannot start any benchmarks until each command supplies a target such as -f net10.0.
Useful? React with 👍 / 👎.
| int key = obj.GetHashCode(); | ||
| _globalObjectList[key] = obj; |
There was a problem hiding this comment.
When two distinct COM wrappers return the same legal 32-bit hash code, assigning by obj.GetHashCode() overwrites the first wrapper, and subsequently removing either wrapper removes the shared entry. The mock's monotonically unique hash codes hide this semantic failure, so the benchmark can recommend a dictionary variant that would lose objects from Core's disposal tracking; use an identity-safe key rather than treating hash codes as unique IDs.
Useful? React with 👍 / 👎.
| /// Simulates disposing a parent with N children. | ||
| /// Expected: O(n²) complexity - performance degrades quadratically | ||
| /// </summary> | ||
| [Benchmark(Baseline = true, Description = "List - Sequential Removal")] |
There was a problem hiding this comment.
This is the only baseline in the class, while the class also contains the unrelated bulk-disposal and mixed-operation scenarios without category grouping. BenchmarkDotNet therefore reports every ratio against sequential List removal rather than comparing each HashSet/Dictionary variant with the corresponding List workload, making the ratios for Scenarios B and C misleading; group the scenarios and assign each List method as its group's baseline.
Useful? React with 👍 / 👎.
1 participant
No description provided.