Skip to content

Create benchmark for Core.RemoveObjectFromList() implementation#476

Draft
jozefizso wants to merge 3 commits into
releases/netoffice_v1.9.10from
dev/448_benchmark_RemoveObjectFromList
Draft

Create benchmark for Core.RemoveObjectFromList() implementation#476
jozefizso wants to merge 3 commits into
releases/netoffice_v1.9.10from
dev/448_benchmark_RemoveObjectFromList

Conversation

@jozefizso
Copy link
Copy Markdown
Member

Implements #448

@jozefizso jozefizso added this to the 1.9.10 milestone Apr 28, 2026
@jozefizso jozefizso self-assigned this Apr 28, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dbea2cffb4

ℹ️ 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".

Comment thread .github/workflows/benchmarks.yml Outdated
Comment thread Tests/NetOffice.Benchmarks/NetOffice.Benchmarks.csproj
jozefizso and others added 3 commits April 28, 2026 21:38
```
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.
@jozefizso jozefizso force-pushed the dev/448_benchmark_RemoveObjectFromList branch from 2e0a960 to e0820de Compare April 28, 2026 19:43
@github-project-automation github-project-automation Bot moved this to Backlog in NetOffice May 12, 2026
Copy link
Copy Markdown
Member Author

Benchmark concern: DictionaryCoreVariant and ConcurrentDictionaryCoreVariant use obj.GetHashCode() as the dictionary key. That is not equivalent to object identity and is not collision-safe. The comments mention an IntPtr key, but the benchmark is measuring hash-code lookup instead, so these results should not be used to justify a dictionary keyed this way.

Copy link
Copy Markdown
Member Author

Benchmark concern: MockCOMObject overrides Equals and GetHashCode, while the production COMObject / COMDynamicObject implementations use the base object behavior. That makes the hash-based variants operate under cleaner equality semantics than the real code path, so the benchmark should use production-equivalent reference identity before the results are considered representative.

Copy link
Copy Markdown
Member Author

Benchmark concern: the mixed-operation benchmark does not run equivalent workloads for all variants. The List case allows duplicate entries in addedObjects, while the hash/dictionary cases check Contains before adding to that tracking list. That changes both the number of tracked objects and the cost profile, so the mixed-operation results are not directly comparable.

Copy link
Copy Markdown
Member Author

Benchmark concern: MemoryBenchmark is described as measuring memory footprint, but the methods allocate the benchmark objects, the tracking List<ICOMObject>, and the collection variant inside the measured operation. Removed objects also remain referenced by the local objects list. This mostly measures allocation throughput for the benchmark scenario, not retained collection footprint after removals.

Copy link
Copy Markdown
Member Author

Benchmark concern: the benchmark currently mixes add cost and remove cost in the same measured method. If the goal is to evaluate RemoveObjectFromList, the variants should be populated in setup and the benchmark should isolate removal. Otherwise the results include construction, growth/resizing, and add-path behavior, which can obscure the removal crossover point.

@jozefizso jozefizso removed this from the 1.9.10 milestone May 21, 2026
@jozefizso jozefizso marked this pull request as draft May 21, 2026 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

1 participant