From 91d7f95dff651ec3ddedfd9364c8db8bca6671f3 Mon Sep 17 00:00:00 2001 From: leecampbell-codeagent Date: Mon, 23 Mar 2026 01:25:01 +0000 Subject: [PATCH 1/7] plan(#161): initial brief from issue --- plan/planning/brief.md | 120 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100644 plan/planning/brief.md diff --git a/plan/planning/brief.md b/plan/planning/brief.md new file mode 100644 index 0000000..a2c6183 --- /dev/null +++ b/plan/planning/brief.md @@ -0,0 +1,120 @@ +# Brief: Issue #161 — Drop netstandard2.0 target, support net8.0+ only + +## Summary + +Remove `netstandard2.0` from the main library's ``, retaining only `net10.0;net9.0;net8.0`. +This eliminates all conditional compilation guards that existed solely to support the older target, simplifies `Bitwise.cs` to use `BitOperations.LeadingZeroCount` unconditionally, and cleans up a branching code path in `HistogramLogReader.cs`. +The spec file `build-system.md` must be updated to reflect the new target list. + +Previous NuGet releases remain available on NuGet.org for consumers on .NET Framework or older runtimes. + +## Category + +`performance` + +Performance is a primary motivation: the netstandard2.0 code path uses a slow, lookup-table-based `LeadingZeroCount` rather than the `BitOperations` hardware intrinsic. +This change also delivers simplification and a reduction in build artefacts, but the performance impact on a hot instrumentation path is the key driver. + +## Affected Files (confirmed by exploration) + +| File | Change required | +|------|----------------| +| `HdrHistogram/HdrHistogram.csproj` | Remove `netstandard2.0` from `` (line 4); delete the netstandard2.0 `PropertyGroup` condition (lines 36-39) | +| `HdrHistogram/Utilities/Bitwise.cs` | Remove `#if NET5_0_OR_GREATER` guards (lines 25-29, 32-44); inline `IntrinsicNumberOfLeadingZeros` body directly into `NumberOfLeadingZeros`; delete the entire `Bitwise.Imperative` class (lines 55-109) | +| `HdrHistogram/HistogramLogReader.cs` | Remove `#if NETSTANDARD2_0` block in `IsComment()` (lines 241-248); keep the `char` overload unconditionally | +| `spec/tech-standards/build-system.md` | Remove `netstandard2.0` from the TargetFrameworks code block (line 25) and from the target table (line 33) | + +No other files in the codebase contain `#if NETSTANDARD` or `#if NET5_0_OR_GREATER` blocks. +Unit-test and benchmarking projects already target `net10.0;net9.0;net8.0` only — no changes needed there. + +## Acceptance Criteria + +- [ ] Library targets `net10.0;net9.0;net8.0` only; `netstandard2.0` is absent from `HdrHistogram.csproj` +- [ ] No `#if NETSTANDARD` or `#if NET5_0_OR_GREATER` conditional compilation remains anywhere in the library +- [ ] `Bitwise.Imperative` class is fully removed +- [ ] `Bitwise.NumberOfLeadingZeros` calls `System.Numerics.BitOperations.LeadingZeroCount` unconditionally +- [ ] `HistogramLogReader.IsComment` uses `line.StartsWith('#')` (char overload) unconditionally +- [ ] All unit tests pass on net8.0, net9.0, and net10.0 +- [ ] `spec/tech-standards/build-system.md` no longer references `netstandard2.0` + +## Test Strategy + +### Existing tests + +No unit tests directly target `Bitwise` or `HistogramLogReader.IsComment` in isolation. +The existing suite exercises both paths indirectly via histogram recording and log-reading tests. +Run the full unit-test suite across all three target frameworks after the changes: + +``` +dotnet test -c Release +``` + +All tests must pass; no new test failures are acceptable. + +### New tests (optional but recommended) + +Consider adding a focused unit test in `HdrHistogram.UnitTests/` that asserts `Bitwise.NumberOfLeadingZeros` returns correct values for a range of inputs (including 0, 1, powers-of-two, and `long.MaxValue`). +This is low-risk but provides a regression anchor if `Bitwise.cs` is touched again. + +There is no need to add tests for `IsComment` — it is a private static helper with trivial logic. + +## Benchmark Strategy + +### Why benchmarks are required + +This is a `performance` category issue. +The stated motivation is that the netstandard2.0 fallback path is slower. +Benchmarks must confirm that the hardware-intrinsic path is indeed faster and that the improvement is observable at both micro and end-to-end level. + +### Existing relevant benchmarks + +`HdrHistogram.Benchmarking/LeadingZeroCount/` already contains benchmarks for this exact area: + +- `LeadingZeroCountBenchmarkBase.cs` — benchmarks `Bitwise.NumberOfLeadingZeros()` (intrinsic path, line 119) and `Bitwise.Imperative.NumberOfLeadingZeros()` (fallback, line 130) + +These benchmarks were written to compare the two implementations. +After `Bitwise.Imperative` is deleted, the `ImperativeImplementation` benchmark method must be removed or updated. + +### Benchmark development plan + +**Phase 1 — Baseline (before any code changes):** + +1. Run the existing `LeadingZeroCount` benchmarks in Release mode against the current code to capture baseline metrics for both the intrinsic and imperative paths. + Record: Mean, StdDev, Allocated, Op/s. +2. If an end-to-end benchmark for `RecordValue` throughput exists (check `Recording/`), run it too. + +**Phase 2 — Implementation:** + +3. Apply the changes described above. +4. Remove the `ImperativeImplementation` benchmark method from `LeadingZeroCountBenchmarkBase.cs` (it references the deleted class). +5. Update `Program.cs` `BenchmarkSwitcher` if the benchmark class is renamed. + +**Phase 3 — Validation:** + +6. Re-run the `LeadingZeroCount` micro-benchmarks and any `Recording` end-to-end benchmarks. +7. Produce a before/after table (Mean, Allocated, Op/s) and include it in the PR description. + +**Metrics that matter:** + +| Metric | Relevance | +|--------|-----------| +| Mean throughput (Op/s) | Primary — shows intrinsic is faster than the lookup table | +| Allocated (bytes) | Secondary — both paths should allocate nothing; confirms no regression | +| GC Gen0/Gen1 collections | Sanity check — should remain zero for both paths | + +A micro-benchmark improvement in `LeadingZeroCount` that does not translate to observable improvement in `RecordValue` throughput is expected (the intrinsic is already used on net8.0+), but the benchmark result still validates correctness and code simplification. + +## Risks and Open Questions + +1. **Benchmark class references `Bitwise.Imperative`** — deleting the class will break the `ImperativeImplementation` benchmark at compile time. + The benchmark must be updated in the same PR. + +2. **NuGet package surface** — removing a target framework is a breaking change for consumers on netstandard2.0. + This is explicitly accepted: "Previous NuGet releases will remain available." + The PR description should note this prominently and suggest a major-version bump or release notes entry. + +3. **No other conditional compilation found** — the grep over the codebase confirmed only two files contain the relevant `#if` blocks. + Low risk of missing anything. + +4. **Documentation file `PropertyGroup` condition** — the existing per-framework Release `PropertyGroup` conditions for net8.0, net9.0, net10.0 (lines 24-34 of the csproj) can be collapsed into a single unconditional `PropertyGroup Condition="'$(Configuration)' == 'Release'"` as shown in `build-system.md`. + This is a tidy-up that is in scope but should be confirmed with the implementer. From 7e84d97508c7944762e6202ce3c5bcb117295b17 Mon Sep 17 00:00:00 2001 From: leecampbell-codeagent Date: Mon, 23 Mar 2026 01:28:26 +0000 Subject: [PATCH 2/7] plan(#161): review brief --- plan/planning/brief-review.md | 82 +++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 plan/planning/brief-review.md diff --git a/plan/planning/brief-review.md b/plan/planning/brief-review.md new file mode 100644 index 0000000..97e73e3 --- /dev/null +++ b/plan/planning/brief-review.md @@ -0,0 +1,82 @@ +# Brief Review: Issue #161 — Drop netstandard2.0 + +## Verdict: Changes required before moving to ready + +The file list, acceptance criteria, and test strategy are solid. +Three issues need resolving. + +--- + +## Issue 1 — Category `performance` is inconsistent with the brief's own analysis (blocking) + +The brief states: + +> "A micro-benchmark improvement in LeadingZeroCount that does not translate to observable improvement in RecordValue throughput is expected (the intrinsic is already used on net8.0+)." + +This concedes the point: **no existing net8.0+ consumer will see any performance improvement**. +The hardware intrinsic path is already taken at compile time via `#if NET5_0_OR_GREATER`. +Removing the fallback does not change the compiled output for any net8.0 or net9.0 or net10.0 build. + +The performance gain only applied to consumers targeting the `netstandard2.0` package on a modern runtime — consumers who are now being dropped, not improved. + +The accurate category is `chore` (or `refactor`). + +**Action required — choose one:** + +- **Option A**: Change category to `chore`. + Remove the entire "Benchmark Strategy" section (benchmarks are not required for `chore`). + Retain the note that `LeadingZeroCountBenchmarkBase.cs` must be updated to remove the `ImperativeImplementation` method so it still compiles. + +- **Option B**: Keep category as `performance` and rewrite the performance rationale. + The argument must be made without contradiction: e.g., "consumers who pinned to the `netstandard2.0` target on .NET 5+ runtimes received the slow fallback path; this change forces them onto the net8.0 target, where they receive the intrinsic." Acknowledge this is a narrow population. + With `performance` retained, the benchmark plan must also be strengthened (see Issue 2). + +--- + +## Issue 2 — End-to-end benchmark is conditional when it should be mandatory (blocking if `performance` is kept) + +The brief says: + +> "If an end-to-end benchmark for `RecordValue` throughput exists (check `Recording/`), run it too." + +`HdrHistogram.Benchmarking/Recording/Recording32BitBenchmark.cs` **was confirmed to exist**. +The spec (`testing-standards.md`) states: "Both levels of benchmark are required." +Conditional language is not acceptable for a `performance` issue. + +**Action required (if `performance` category is retained):** + +Replace the conditional phrasing with an explicit instruction: + +> "Run `Recording32BitBenchmark` as the end-to-end benchmark. +> Include before/after results in the PR description alongside the `LeadingZeroCount` micro-benchmark results." + +--- + +## Issue 3 — PropertyGroup consolidation scope is unresolved (minor, non-blocking) + +Risk item 4 notes that the three per-framework `Release` `PropertyGroup` conditions could be collapsed into one unconditional block, but says "should be confirmed with the implementer." + +A brief should not leave scope ambiguous. +The implementer cannot resolve this themselves — it must be decided before implementation begins. + +**Action required:** + +Make an explicit decision and state it clearly, e.g.: + +> "Collapsing the three per-framework Release `PropertyGroup` conditions into a single `Condition="'$(Configuration)' == 'Release'"` block is **in scope** for this PR." + +or: + +> "Collapsing the Release `PropertyGroup` conditions is **out of scope**; a follow-up chore can address it." + +--- + +## What is correct and does not need changing + +- All four affected files exist at the stated paths. +- Line-level descriptions of the required changes are accurate. +- The claim that only two source files contain the relevant `#if` blocks is confirmed. +- Unit test and benchmarking projects already target `net10.0;net9.0;net8.0` — no changes needed there. +- Acceptance criteria are measurable and complete. +- The note that `ImperativeImplementation` in `LeadingZeroCountBenchmarkBase.cs` must be removed is correct and necessary. +- The NuGet breaking-change risk is correctly identified. From 7720d03862374b832ce11f2ee2b617576e95d59e Mon Sep 17 00:00:00 2001 From: leecampbell-codeagent Date: Mon, 23 Mar 2026 01:29:44 +0000 Subject: [PATCH 3/7] plan(#161): apply brief review feedback --- plan/planning/brief-review.md | 82 ----------------------------------- plan/planning/brief.md | 64 +++++---------------------- 2 files changed, 10 insertions(+), 136 deletions(-) delete mode 100644 plan/planning/brief-review.md diff --git a/plan/planning/brief-review.md b/plan/planning/brief-review.md deleted file mode 100644 index 97e73e3..0000000 --- a/plan/planning/brief-review.md +++ /dev/null @@ -1,82 +0,0 @@ -# Brief Review: Issue #161 — Drop netstandard2.0 - -## Verdict: Changes required before moving to ready - -The file list, acceptance criteria, and test strategy are solid. -Three issues need resolving. - ---- - -## Issue 1 — Category `performance` is inconsistent with the brief's own analysis (blocking) - -The brief states: - -> "A micro-benchmark improvement in LeadingZeroCount that does not translate to observable improvement in RecordValue throughput is expected (the intrinsic is already used on net8.0+)." - -This concedes the point: **no existing net8.0+ consumer will see any performance improvement**. -The hardware intrinsic path is already taken at compile time via `#if NET5_0_OR_GREATER`. -Removing the fallback does not change the compiled output for any net8.0 or net9.0 or net10.0 build. - -The performance gain only applied to consumers targeting the `netstandard2.0` package on a modern runtime — consumers who are now being dropped, not improved. - -The accurate category is `chore` (or `refactor`). - -**Action required — choose one:** - -- **Option A**: Change category to `chore`. - Remove the entire "Benchmark Strategy" section (benchmarks are not required for `chore`). - Retain the note that `LeadingZeroCountBenchmarkBase.cs` must be updated to remove the `ImperativeImplementation` method so it still compiles. - -- **Option B**: Keep category as `performance` and rewrite the performance rationale. - The argument must be made without contradiction: e.g., "consumers who pinned to the `netstandard2.0` target on .NET 5+ runtimes received the slow fallback path; this change forces them onto the net8.0 target, where they receive the intrinsic." Acknowledge this is a narrow population. - With `performance` retained, the benchmark plan must also be strengthened (see Issue 2). - ---- - -## Issue 2 — End-to-end benchmark is conditional when it should be mandatory (blocking if `performance` is kept) - -The brief says: - -> "If an end-to-end benchmark for `RecordValue` throughput exists (check `Recording/`), run it too." - -`HdrHistogram.Benchmarking/Recording/Recording32BitBenchmark.cs` **was confirmed to exist**. -The spec (`testing-standards.md`) states: "Both levels of benchmark are required." -Conditional language is not acceptable for a `performance` issue. - -**Action required (if `performance` category is retained):** - -Replace the conditional phrasing with an explicit instruction: - -> "Run `Recording32BitBenchmark` as the end-to-end benchmark. -> Include before/after results in the PR description alongside the `LeadingZeroCount` micro-benchmark results." - ---- - -## Issue 3 — PropertyGroup consolidation scope is unresolved (minor, non-blocking) - -Risk item 4 notes that the three per-framework `Release` `PropertyGroup` conditions could be collapsed into one unconditional block, but says "should be confirmed with the implementer." - -A brief should not leave scope ambiguous. -The implementer cannot resolve this themselves — it must be decided before implementation begins. - -**Action required:** - -Make an explicit decision and state it clearly, e.g.: - -> "Collapsing the three per-framework Release `PropertyGroup` conditions into a single `Condition="'$(Configuration)' == 'Release'"` block is **in scope** for this PR." - -or: - -> "Collapsing the Release `PropertyGroup` conditions is **out of scope**; a follow-up chore can address it." - ---- - -## What is correct and does not need changing - -- All four affected files exist at the stated paths. -- Line-level descriptions of the required changes are accurate. -- The claim that only two source files contain the relevant `#if` blocks is confirmed. -- Unit test and benchmarking projects already target `net10.0;net9.0;net8.0` — no changes needed there. -- Acceptance criteria are measurable and complete. -- The note that `ImperativeImplementation` in `LeadingZeroCountBenchmarkBase.cs` must be removed is correct and necessary. -- The NuGet breaking-change risk is correctly identified. diff --git a/plan/planning/brief.md b/plan/planning/brief.md index a2c6183..7737b4f 100644 --- a/plan/planning/brief.md +++ b/plan/planning/brief.md @@ -4,25 +4,28 @@ Remove `netstandard2.0` from the main library's ``, retaining only `net10.0;net9.0;net8.0`. This eliminates all conditional compilation guards that existed solely to support the older target, simplifies `Bitwise.cs` to use `BitOperations.LeadingZeroCount` unconditionally, and cleans up a branching code path in `HistogramLogReader.cs`. +The three per-framework Release `PropertyGroup` conditions in the csproj are collapsed into a single unconditional `PropertyGroup` as part of this change. The spec file `build-system.md` must be updated to reflect the new target list. Previous NuGet releases remain available on NuGet.org for consumers on .NET Framework or older runtimes. ## Category -`performance` +`chore` -Performance is a primary motivation: the netstandard2.0 code path uses a slow, lookup-table-based `LeadingZeroCount` rather than the `BitOperations` hardware intrinsic. -This change also delivers simplification and a reduction in build artefacts, but the performance impact on a hot instrumentation path is the key driver. +This is a maintenance and simplification task. +The hardware-intrinsic `LeadingZeroCount` path (`#if NET5_0_OR_GREATER`) is already compiled in for all net8.0+ builds; removing the fallback does not change the compiled output for any net8.0, net9.0, or net10.0 consumer. +No performance improvement is delivered to existing net8.0+ consumers — the change reduces build artefacts, removes dead code, and simplifies the codebase. ## Affected Files (confirmed by exploration) | File | Change required | |------|----------------| -| `HdrHistogram/HdrHistogram.csproj` | Remove `netstandard2.0` from `` (line 4); delete the netstandard2.0 `PropertyGroup` condition (lines 36-39) | +| `HdrHistogram/HdrHistogram.csproj` | Remove `netstandard2.0` from `` (line 4); delete the netstandard2.0 `PropertyGroup` condition (lines 36-39); collapse the three per-framework Release `PropertyGroup` conditions (lines 24-34) into a single `Condition="'$(Configuration)' == 'Release'"` block | | `HdrHistogram/Utilities/Bitwise.cs` | Remove `#if NET5_0_OR_GREATER` guards (lines 25-29, 32-44); inline `IntrinsicNumberOfLeadingZeros` body directly into `NumberOfLeadingZeros`; delete the entire `Bitwise.Imperative` class (lines 55-109) | | `HdrHistogram/HistogramLogReader.cs` | Remove `#if NETSTANDARD2_0` block in `IsComment()` (lines 241-248); keep the `char` overload unconditionally | | `spec/tech-standards/build-system.md` | Remove `netstandard2.0` from the TargetFrameworks code block (line 25) and from the target table (line 33) | +| `HdrHistogram.Benchmarking/LeadingZeroCount/LeadingZeroCountBenchmarkBase.cs` | Remove the `ImperativeImplementation` benchmark method (references deleted `Bitwise.Imperative` class) | No other files in the codebase contain `#if NETSTANDARD` or `#if NET5_0_OR_GREATER` blocks. Unit-test and benchmarking projects already target `net10.0;net9.0;net8.0` only — no changes needed there. @@ -34,6 +37,8 @@ Unit-test and benchmarking projects already target `net10.0;net9.0;net8.0` only - [ ] `Bitwise.Imperative` class is fully removed - [ ] `Bitwise.NumberOfLeadingZeros` calls `System.Numerics.BitOperations.LeadingZeroCount` unconditionally - [ ] `HistogramLogReader.IsComment` uses `line.StartsWith('#')` (char overload) unconditionally +- [ ] The three per-framework Release `PropertyGroup` conditions are collapsed into a single `Condition="'$(Configuration)' == 'Release'"` block +- [ ] `LeadingZeroCountBenchmarkBase.cs` no longer references `Bitwise.Imperative`; the benchmarking project compiles cleanly - [ ] All unit tests pass on net8.0, net9.0, and net10.0 - [ ] `spec/tech-standards/build-system.md` no longer references `netstandard2.0` @@ -58,56 +63,10 @@ This is low-risk but provides a regression anchor if `Bitwise.cs` is touched aga There is no need to add tests for `IsComment` — it is a private static helper with trivial logic. -## Benchmark Strategy - -### Why benchmarks are required - -This is a `performance` category issue. -The stated motivation is that the netstandard2.0 fallback path is slower. -Benchmarks must confirm that the hardware-intrinsic path is indeed faster and that the improvement is observable at both micro and end-to-end level. - -### Existing relevant benchmarks - -`HdrHistogram.Benchmarking/LeadingZeroCount/` already contains benchmarks for this exact area: - -- `LeadingZeroCountBenchmarkBase.cs` — benchmarks `Bitwise.NumberOfLeadingZeros()` (intrinsic path, line 119) and `Bitwise.Imperative.NumberOfLeadingZeros()` (fallback, line 130) - -These benchmarks were written to compare the two implementations. -After `Bitwise.Imperative` is deleted, the `ImperativeImplementation` benchmark method must be removed or updated. - -### Benchmark development plan - -**Phase 1 — Baseline (before any code changes):** - -1. Run the existing `LeadingZeroCount` benchmarks in Release mode against the current code to capture baseline metrics for both the intrinsic and imperative paths. - Record: Mean, StdDev, Allocated, Op/s. -2. If an end-to-end benchmark for `RecordValue` throughput exists (check `Recording/`), run it too. - -**Phase 2 — Implementation:** - -3. Apply the changes described above. -4. Remove the `ImperativeImplementation` benchmark method from `LeadingZeroCountBenchmarkBase.cs` (it references the deleted class). -5. Update `Program.cs` `BenchmarkSwitcher` if the benchmark class is renamed. - -**Phase 3 — Validation:** - -6. Re-run the `LeadingZeroCount` micro-benchmarks and any `Recording` end-to-end benchmarks. -7. Produce a before/after table (Mean, Allocated, Op/s) and include it in the PR description. - -**Metrics that matter:** - -| Metric | Relevance | -|--------|-----------| -| Mean throughput (Op/s) | Primary — shows intrinsic is faster than the lookup table | -| Allocated (bytes) | Secondary — both paths should allocate nothing; confirms no regression | -| GC Gen0/Gen1 collections | Sanity check — should remain zero for both paths | - -A micro-benchmark improvement in `LeadingZeroCount` that does not translate to observable improvement in `RecordValue` throughput is expected (the intrinsic is already used on net8.0+), but the benchmark result still validates correctness and code simplification. - ## Risks and Open Questions 1. **Benchmark class references `Bitwise.Imperative`** — deleting the class will break the `ImperativeImplementation` benchmark at compile time. - The benchmark must be updated in the same PR. + The benchmark must be updated in the same PR (listed in Affected Files above). 2. **NuGet package surface** — removing a target framework is a breaking change for consumers on netstandard2.0. This is explicitly accepted: "Previous NuGet releases will remain available." @@ -115,6 +74,3 @@ A micro-benchmark improvement in `LeadingZeroCount` that does not translate to o 3. **No other conditional compilation found** — the grep over the codebase confirmed only two files contain the relevant `#if` blocks. Low risk of missing anything. - -4. **Documentation file `PropertyGroup` condition** — the existing per-framework Release `PropertyGroup` conditions for net8.0, net9.0, net10.0 (lines 24-34 of the csproj) can be collapsed into a single unconditional `PropertyGroup Condition="'$(Configuration)' == 'Release'"` as shown in `build-system.md`. - This is a tidy-up that is in scope but should be confirmed with the implementer. From cbe793cfa88a19cfb428101f90ba1407c4c5ba83 Mon Sep 17 00:00:00 2001 From: leecampbell-codeagent Date: Mon, 23 Mar 2026 01:31:35 +0000 Subject: [PATCH 4/7] plan(#161): review brief --- plan/{planning => ready}/brief.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename plan/{planning => ready}/brief.md (100%) diff --git a/plan/planning/brief.md b/plan/ready/brief.md similarity index 100% rename from plan/planning/brief.md rename to plan/ready/brief.md From 43b4cdb373fcc1d25af1ae49145c8326c22bbf93 Mon Sep 17 00:00:00 2001 From: leecampbell-codeagent Date: Mon, 23 Mar 2026 01:33:43 +0000 Subject: [PATCH 5/7] plan(#161): create task breakdown --- plan/ready/task.md | 67 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 plan/ready/task.md diff --git a/plan/ready/task.md b/plan/ready/task.md new file mode 100644 index 0000000..af3c517 --- /dev/null +++ b/plan/ready/task.md @@ -0,0 +1,67 @@ +# Task List: Issue #161 — Drop netstandard2.0 target, support net8.0+ only + +## Implementation Changes + +- [ ] **`HdrHistogram/HdrHistogram.csproj` line 4** — Remove `netstandard2.0` from ``, leaving `net10.0;net9.0;net8.0`. + - **Why:** Drops the legacy target entirely. + - **Verify:** `` value contains no `netstandard2.0` token. + +- [ ] **`HdrHistogram/HdrHistogram.csproj` lines 24–34** — Collapse the three per-framework Release `PropertyGroup` blocks (one each for `net8.0`, `net9.0`, `net10.0`) into a single `PropertyGroup Condition="'$(Configuration)' == 'Release'"` block with a single `` element that resolves via `$(TargetFramework)`. + - **Why:** Eliminates duplicated XML and the framework-specific condition strings. + - **Verify:** Only one Release `PropertyGroup` exists; it contains no `$(TargetFramework)` literals in the condition string. + +- [ ] **`HdrHistogram/HdrHistogram.csproj` lines 36–39** — Delete the `netstandard2.0` `PropertyGroup` block (the one setting `DefineConstants` to `RELEASE;NETSTANDARD2_0`). + - **Why:** The constant `NETSTANDARD2_0` is no longer needed once the target is removed. + - **Verify:** No `PropertyGroup` referencing `netstandard2.0` or `NETSTANDARD2_0` remains in the file. + +- [ ] **`HdrHistogram/Utilities/Bitwise.cs` lines 23–44** — Simplify `NumberOfLeadingZeros(long)` to call `System.Numerics.BitOperations.LeadingZeroCount((ulong)value)` directly; remove the `#if NET5_0_OR_GREATER` / `#else` / `#endif` guards and delete the private `IntrinsicNumberOfLeadingZeros` helper. + - **Why:** All supported targets (net8.0+) provide `BitOperations.LeadingZeroCount`; the conditional dispatch is dead code. + - **Verify:** `NumberOfLeadingZeros` body is a single `return System.Numerics.BitOperations.LeadingZeroCount((ulong)value);` statement; no `#if` directives remain in the method or immediately around it. + +- [ ] **`HdrHistogram/Utilities/Bitwise.cs` lines 55–109** — Delete the entire `Bitwise.Imperative` nested public static class (including the `Lookup` table, `NumberOfLeadingZeros`, `NumberOfLeadingZerosLong`, and `Log2` methods). + - **Why:** The imperative fallback path is unreachable on net8.0+; removing it eliminates dead code and the public surface that the benchmark references. + - **Verify:** No `class Imperative` or `Bitwise.Imperative` identifier exists anywhere in the solution. + +- [ ] **`HdrHistogram/HistogramLogReader.cs` lines 241–248** — Remove the `#if NETSTANDARD2_0` / `#else` / `#endif` block inside `IsComment(string line)`, keeping only `return line.StartsWith('#');`. + - **Why:** The `char` overload of `StartsWith` is available on all net8.0+ targets; the string-overload fallback is dead code. + - **Verify:** `IsComment` contains no `#if` directives; the method body is `return line.StartsWith('#');`. + +- [ ] **`HdrHistogram.Benchmarking/LeadingZeroCount/LeadingZeroCountBenchmarkBase.cs`** — Remove the `"Imperative"` entry from the validation dictionary (line 56) and delete the `ImperativeImplementation()` benchmark method (lines 124–133). + - **Why:** Both reference `Bitwise.Imperative` which will no longer exist; leaving them causes a compile error. + - **Verify:** `dotnet build HdrHistogram.Benchmarking/ -c Release` exits with code 0; no reference to `Bitwise.Imperative` remains in the file. + +## Unit Tests + +- [ ] **`HdrHistogram.UnitTests/`** — Add a focused unit test class `BitwiseTests` (e.g. `HdrHistogram.UnitTests/Utilities/BitwiseTests.cs`) that asserts `Bitwise.NumberOfLeadingZeros` returns correct results for representative inputs: `0`, `1`, `2`, powers of two up to 2⁶², and `long.MaxValue`. + - **Why:** No existing test directly covers `Bitwise`; this provides a regression anchor if the method is ever touched again. + - **Verify:** Test class exists; `dotnet test -c Release` reports the new tests as passing on all three target frameworks. + +- [ ] **Run the full unit-test suite** — Execute `dotnet test -c Release` across all three target frameworks (net8.0, net9.0, net10.0). + - **Why:** Confirms that removing the netstandard2.0 conditional paths has not broken any indirect consumer of `Bitwise` or `HistogramLogReader`. + - **Verify:** Zero test failures; zero skipped tests that were previously passing. + +## Documentation + +- [ ] **`spec/tech-standards/build-system.md` line 25** — Remove `netstandard2.0` from the `` code block example. + - **Why:** The spec must reflect the actual supported targets. + - **Verify:** The code block contains only `net10.0;net9.0;net8.0`. + +- [ ] **`spec/tech-standards/build-system.md` line 33** — Delete the `| \`netstandard2.0\` | Broad compatibility (.NET Framework 4.6.1+, .NET Core 2.0+) |` row from the target table. + - **Why:** The target no longer exists; the row is misleading. + - **Verify:** No mention of `netstandard2.0` remains anywhere in `build-system.md`. + +--- + +## Acceptance Criteria Cross-Reference + +| Acceptance criterion (from brief) | Covered by task | +|---|---| +| Library targets `net10.0;net9.0;net8.0` only; `netstandard2.0` absent from `.csproj` | Task 1 (TargetFrameworks) | +| No `#if NETSTANDARD` or `#if NET5_0_OR_GREATER` conditional compilation in library | Tasks 4, 6 (Bitwise.cs, HistogramLogReader.cs) | +| `Bitwise.Imperative` class fully removed | Task 5 | +| `Bitwise.NumberOfLeadingZeros` calls `BitOperations.LeadingZeroCount` unconditionally | Task 4 | +| `HistogramLogReader.IsComment` uses `line.StartsWith('#')` unconditionally | Task 6 | +| Three per-framework Release `PropertyGroup` conditions collapsed into one | Task 2 | +| `LeadingZeroCountBenchmarkBase.cs` no longer references `Bitwise.Imperative`; benchmarking project compiles | Task 7 | +| All unit tests pass on net8.0, net9.0, net10.0 | Tasks 8 (new tests), 9 (full suite) | +| `spec/tech-standards/build-system.md` no longer references `netstandard2.0` | Tasks 10, 11 | From ddc4c7565d991a850a762050e16b3ae7bd31935a Mon Sep 17 00:00:00 2001 From: leecampbell-codeagent Date: Mon, 23 Mar 2026 01:40:50 +0000 Subject: [PATCH 6/7] feat(#161): implement tasks --- .../LeadingZeroCountBenchmarkBase.cs | 12 --- .../Utilities/BitwiseTests.cs | 45 ++++++++++ HdrHistogram/HdrHistogram.csproj | 23 ++--- HdrHistogram/HistogramLogReader.cs | 4 - HdrHistogram/Utilities/Bitwise.cs | 85 +------------------ plan/{ready => done}/brief.md | 0 plan/{ready => done}/task.md | 22 ++--- spec/tech-standards/build-system.md | 3 +- 8 files changed, 63 insertions(+), 131 deletions(-) create mode 100644 HdrHistogram.UnitTests/Utilities/BitwiseTests.cs rename plan/{ready => done}/brief.md (100%) rename plan/{ready => done}/task.md (88%) diff --git a/HdrHistogram.Benchmarking/LeadingZeroCount/LeadingZeroCountBenchmarkBase.cs b/HdrHistogram.Benchmarking/LeadingZeroCount/LeadingZeroCountBenchmarkBase.cs index 23969ca..439a877 100644 --- a/HdrHistogram.Benchmarking/LeadingZeroCount/LeadingZeroCountBenchmarkBase.cs +++ b/HdrHistogram.Benchmarking/LeadingZeroCount/LeadingZeroCountBenchmarkBase.cs @@ -53,7 +53,6 @@ public void OneOffValidationOfImplementations() var functions = new Dictionary> { {"CurrentImpl", Bitwise.NumberOfLeadingZeros}, - {"Imperative", Bitwise.Imperative.NumberOfLeadingZeros}, {"IfAndShift", LeadingZeroCount.IfAndShift.GetLeadingZeroCount}, //{"MathLog", LeadingZeroCount.MathLog.GetLeadingZeroCount}, {"StringManipulation", LeadingZeroCount.StringManipulation.GetLeadingZeroCount}, @@ -121,17 +120,6 @@ public int CurrentImplementation() return sum; } - [Benchmark(OperationsPerInvoke = TEST_VALUE_LENGTH)] - public int ImperativeImplementation() - { - var sum = 0; - for (int i = 0; i < _testValues.Length; i++) - { - sum += Bitwise.Imperative.NumberOfLeadingZeros(_testValues[i]); - } - return sum; - } - [Benchmark(OperationsPerInvoke = TEST_VALUE_LENGTH)] public int IfAndShift() { diff --git a/HdrHistogram.UnitTests/Utilities/BitwiseTests.cs b/HdrHistogram.UnitTests/Utilities/BitwiseTests.cs new file mode 100644 index 0000000..dfadecc --- /dev/null +++ b/HdrHistogram.UnitTests/Utilities/BitwiseTests.cs @@ -0,0 +1,45 @@ +/* + * This is a .NET port of the original Java version, which was written by + * Gil Tene as described in + * https://github.com/HdrHistogram/HdrHistogram + * and released to the public domain, as explained at + * http://creativecommons.org/publicdomain/zero/1.0/ + */ + +using HdrHistogram.Utilities; +using Xunit; + +namespace HdrHistogram.UnitTests.Utilities +{ + public class BitwiseTests + { + [Theory] + [InlineData(0L, 64)] + [InlineData(1L, 63)] + [InlineData(2L, 62)] + [InlineData(4L, 61)] + [InlineData(8L, 60)] + [InlineData(16L, 59)] + [InlineData(32L, 58)] + [InlineData(64L, 57)] + [InlineData(128L, 56)] + [InlineData(256L, 55)] + [InlineData(512L, 54)] + [InlineData(1024L, 53)] + [InlineData(2048L, 52)] + [InlineData(4096L, 51)] + [InlineData(8192L, 50)] + [InlineData(16384L, 49)] + [InlineData(32768L, 48)] + [InlineData(65536L, 47)] + [InlineData(1L << 30, 33)] + [InlineData(1L << 31, 32)] + [InlineData(1L << 32, 31)] + [InlineData(1L << 62, 1)] + [InlineData(long.MaxValue, 1)] + public void NumberOfLeadingZeros_ReturnsCorrectValue(long value, int expected) + { + Assert.Equal(expected, Bitwise.NumberOfLeadingZeros(value)); + } + } +} diff --git a/HdrHistogram/HdrHistogram.csproj b/HdrHistogram/HdrHistogram.csproj index 26279d1..aae3933 100644 --- a/HdrHistogram/HdrHistogram.csproj +++ b/HdrHistogram/HdrHistogram.csproj @@ -1,7 +1,7 @@ - + - net10.0;net9.0;net8.0;netstandard2.0 + net10.0;net9.0;net8.0 HdrHistogram supports low latency recording and analyzing of sampled data value counts across a configurable integer value range with configurable value precision within the range. Gil Tene, Lee Campbell Net 8.0 release @@ -21,21 +21,8 @@ - - bin\Release\net8.0\HdrHistogram.xml + + bin\Release\$(TargetFramework)\HdrHistogram.xml - - bin\Release\net9.0\HdrHistogram.xml - - - - bin\Release\net10.0\HdrHistogram.xml - - - - bin\Release\netstandard2.0\HdrHistogram.xml - RELEASE;NETSTANDARD2_0 - - - \ No newline at end of file + diff --git a/HdrHistogram/HistogramLogReader.cs b/HdrHistogram/HistogramLogReader.cs index c861b59..000cfef 100644 --- a/HdrHistogram/HistogramLogReader.cs +++ b/HdrHistogram/HistogramLogReader.cs @@ -240,11 +240,7 @@ private IEnumerable ReadLines() private static bool IsComment(string line) { -#if NETSTANDARD2_0 - return line.StartsWith("#", StringComparison.Ordinal); -#else return line.StartsWith('#'); -#endif } private static bool IsStartTime(string line) diff --git a/HdrHistogram/Utilities/Bitwise.cs b/HdrHistogram/Utilities/Bitwise.cs index 9f4782e..fd5431b 100644 --- a/HdrHistogram/Utilities/Bitwise.cs +++ b/HdrHistogram/Utilities/Bitwise.cs @@ -22,90 +22,7 @@ public static class Bitwise /// The number of leading zeros. public static int NumberOfLeadingZeros(long value) { -#if NET5_0_OR_GREATER - return IntrinsicNumberOfLeadingZeros(value); -#else - return Imperative.NumberOfLeadingZeros(value); -#endif - } - -#if NET5_0_OR_GREATER - /// - /// Returns the Leading Zero Count (lzc) of the for its binary representation. - /// - /// The value to find the number of leading zeros - /// The number of leading zeros. - private static int IntrinsicNumberOfLeadingZeros(long value) - { - ulong testValue = (ulong)value; - return System.Numerics.BitOperations.LeadingZeroCount(testValue); - - } -#endif - - //Code has been tested and taken from : - //http://stackoverflow.com/questions/9543410/i-dont-think-numberofleadingzeroslong-i-in-long-java-is-based-floorlog2x/9543537#9543537 - //http://stackoverflow.com/questions/21888140/de-bruijn-algorithm-binary-digit-count-64bits-c-sharp/21888542#21888542 - //http://stackoverflow.com/questions/15967240/fastest-implementation-of-log2int-and-log2float - //http://graphics.stanford.edu/~seander/bithacks.html#IntegerLogObvious - - /// - /// Imperative implementation of the LeadingZeroCount, when access to the System.Numerics.BitOperations.LeadingZeroCount(ulong) is not available. - /// - public static class Imperative - { - private static readonly int[] Lookup; - - static Imperative() - { - Lookup = new int[256]; - for (int i = 1; i < 256; ++i) - { - Lookup[i] = (int)(Math.Log(i) / Math.Log(2)); - } - } - - /// - /// Returns the Leading Zero Count (lzc) of the for its binary representation. - /// - /// The value to find the number of leading zeros - /// The number of leading zeros. - public static int NumberOfLeadingZeros(long value) - { - //Optimisation for 32 bit values. So values under 00:16:41.0 when measuring with Stopwatch.GetTimestamp()*, we will hit a fast path. - // * as at writing on Win10 .NET 4.6 - if (value < int.MaxValue) - return 63 - Log2((int)value); - return NumberOfLeadingZerosLong(value); - } - - private static int NumberOfLeadingZerosLong(long value) - { - // Code from http://stackoverflow.com/questions/9543410/i-dont-think-numberofleadingzeroslong-i-in-long-java-is-based-floorlog2x/9543537#9543537 - - //--Already checked that values here are over int.MaxValue, i.e. !=0 - // HD, Figure 5-6 - //if (value == 0) - // return 64; - var n = 1; - // >>> in Java is a "unsigned bit shift", to do the same in C# we use >> (but it HAS to be an unsigned int) - var x = (uint)(value >> 32); - if (x == 0) { n += 32; x = (uint)value; } - if (x >> 16 == 0) { n += 16; x <<= 16; } - if (x >> 24 == 0) { n += 8; x <<= 8; } - if (x >> 28 == 0) { n += 4; x <<= 4; } - if (x >> 30 == 0) { n += 2; x <<= 2; } - n -= (int)(x >> 31); - return n; - } - - private static int Log2(int i) - { - if (i >= 0x1000000) { return Lookup[i >> 24] + 24; } - if (i >= 0x10000) { return Lookup[i >> 16] + 16; } - if (i >= 0x100) { return Lookup[i >> 8] + 8; } - return Lookup[i]; - } + return System.Numerics.BitOperations.LeadingZeroCount((ulong)value); } } } diff --git a/plan/ready/brief.md b/plan/done/brief.md similarity index 100% rename from plan/ready/brief.md rename to plan/done/brief.md diff --git a/plan/ready/task.md b/plan/done/task.md similarity index 88% rename from plan/ready/task.md rename to plan/done/task.md index af3c517..7079ea3 100644 --- a/plan/ready/task.md +++ b/plan/done/task.md @@ -2,51 +2,51 @@ ## Implementation Changes -- [ ] **`HdrHistogram/HdrHistogram.csproj` line 4** — Remove `netstandard2.0` from ``, leaving `net10.0;net9.0;net8.0`. +- [x] **`HdrHistogram/HdrHistogram.csproj` line 4** — Remove `netstandard2.0` from ``, leaving `net10.0;net9.0;net8.0`. - **Why:** Drops the legacy target entirely. - **Verify:** `` value contains no `netstandard2.0` token. -- [ ] **`HdrHistogram/HdrHistogram.csproj` lines 24–34** — Collapse the three per-framework Release `PropertyGroup` blocks (one each for `net8.0`, `net9.0`, `net10.0`) into a single `PropertyGroup Condition="'$(Configuration)' == 'Release'"` block with a single `` element that resolves via `$(TargetFramework)`. +- [x] **`HdrHistogram/HdrHistogram.csproj` lines 24–34** — Collapse the three per-framework Release `PropertyGroup` blocks (one each for `net8.0`, `net9.0`, `net10.0`) into a single `PropertyGroup Condition="'$(Configuration)' == 'Release'"` block with a single `` element that resolves via `$(TargetFramework)`. - **Why:** Eliminates duplicated XML and the framework-specific condition strings. - **Verify:** Only one Release `PropertyGroup` exists; it contains no `$(TargetFramework)` literals in the condition string. -- [ ] **`HdrHistogram/HdrHistogram.csproj` lines 36–39** — Delete the `netstandard2.0` `PropertyGroup` block (the one setting `DefineConstants` to `RELEASE;NETSTANDARD2_0`). +- [x] **`HdrHistogram/HdrHistogram.csproj` lines 36–39** — Delete the `netstandard2.0` `PropertyGroup` block (the one setting `DefineConstants` to `RELEASE;NETSTANDARD2_0`). - **Why:** The constant `NETSTANDARD2_0` is no longer needed once the target is removed. - **Verify:** No `PropertyGroup` referencing `netstandard2.0` or `NETSTANDARD2_0` remains in the file. -- [ ] **`HdrHistogram/Utilities/Bitwise.cs` lines 23–44** — Simplify `NumberOfLeadingZeros(long)` to call `System.Numerics.BitOperations.LeadingZeroCount((ulong)value)` directly; remove the `#if NET5_0_OR_GREATER` / `#else` / `#endif` guards and delete the private `IntrinsicNumberOfLeadingZeros` helper. +- [x] **`HdrHistogram/Utilities/Bitwise.cs` lines 23–44** — Simplify `NumberOfLeadingZeros(long)` to call `System.Numerics.BitOperations.LeadingZeroCount((ulong)value)` directly; remove the `#if NET5_0_OR_GREATER` / `#else` / `#endif` guards and delete the private `IntrinsicNumberOfLeadingZeros` helper. - **Why:** All supported targets (net8.0+) provide `BitOperations.LeadingZeroCount`; the conditional dispatch is dead code. - **Verify:** `NumberOfLeadingZeros` body is a single `return System.Numerics.BitOperations.LeadingZeroCount((ulong)value);` statement; no `#if` directives remain in the method or immediately around it. -- [ ] **`HdrHistogram/Utilities/Bitwise.cs` lines 55–109** — Delete the entire `Bitwise.Imperative` nested public static class (including the `Lookup` table, `NumberOfLeadingZeros`, `NumberOfLeadingZerosLong`, and `Log2` methods). +- [x] **`HdrHistogram/Utilities/Bitwise.cs` lines 55–109** — Delete the entire `Bitwise.Imperative` nested public static class (including the `Lookup` table, `NumberOfLeadingZeros`, `NumberOfLeadingZerosLong`, and `Log2` methods). - **Why:** The imperative fallback path is unreachable on net8.0+; removing it eliminates dead code and the public surface that the benchmark references. - **Verify:** No `class Imperative` or `Bitwise.Imperative` identifier exists anywhere in the solution. -- [ ] **`HdrHistogram/HistogramLogReader.cs` lines 241–248** — Remove the `#if NETSTANDARD2_0` / `#else` / `#endif` block inside `IsComment(string line)`, keeping only `return line.StartsWith('#');`. +- [x] **`HdrHistogram/HistogramLogReader.cs` lines 241–248** — Remove the `#if NETSTANDARD2_0` / `#else` / `#endif` block inside `IsComment(string line)`, keeping only `return line.StartsWith('#');`. - **Why:** The `char` overload of `StartsWith` is available on all net8.0+ targets; the string-overload fallback is dead code. - **Verify:** `IsComment` contains no `#if` directives; the method body is `return line.StartsWith('#');`. -- [ ] **`HdrHistogram.Benchmarking/LeadingZeroCount/LeadingZeroCountBenchmarkBase.cs`** — Remove the `"Imperative"` entry from the validation dictionary (line 56) and delete the `ImperativeImplementation()` benchmark method (lines 124–133). +- [x] **`HdrHistogram.Benchmarking/LeadingZeroCount/LeadingZeroCountBenchmarkBase.cs`** — Remove the `"Imperative"` entry from the validation dictionary (line 56) and delete the `ImperativeImplementation()` benchmark method (lines 124–133). - **Why:** Both reference `Bitwise.Imperative` which will no longer exist; leaving them causes a compile error. - **Verify:** `dotnet build HdrHistogram.Benchmarking/ -c Release` exits with code 0; no reference to `Bitwise.Imperative` remains in the file. ## Unit Tests -- [ ] **`HdrHistogram.UnitTests/`** — Add a focused unit test class `BitwiseTests` (e.g. `HdrHistogram.UnitTests/Utilities/BitwiseTests.cs`) that asserts `Bitwise.NumberOfLeadingZeros` returns correct results for representative inputs: `0`, `1`, `2`, powers of two up to 2⁶², and `long.MaxValue`. +- [x] **`HdrHistogram.UnitTests/`** — Add a focused unit test class `BitwiseTests` (e.g. `HdrHistogram.UnitTests/Utilities/BitwiseTests.cs`) that asserts `Bitwise.NumberOfLeadingZeros` returns correct results for representative inputs: `0`, `1`, `2`, powers of two up to 2⁶², and `long.MaxValue`. - **Why:** No existing test directly covers `Bitwise`; this provides a regression anchor if the method is ever touched again. - **Verify:** Test class exists; `dotnet test -c Release` reports the new tests as passing on all three target frameworks. -- [ ] **Run the full unit-test suite** — Execute `dotnet test -c Release` across all three target frameworks (net8.0, net9.0, net10.0). +- [x] **Run the full unit-test suite** — Execute `dotnet test -c Release` across all three target frameworks (net8.0, net9.0, net10.0). - **Why:** Confirms that removing the netstandard2.0 conditional paths has not broken any indirect consumer of `Bitwise` or `HistogramLogReader`. - **Verify:** Zero test failures; zero skipped tests that were previously passing. ## Documentation -- [ ] **`spec/tech-standards/build-system.md` line 25** — Remove `netstandard2.0` from the `` code block example. +- [x] **`spec/tech-standards/build-system.md` line 25** — Remove `netstandard2.0` from the `` code block example. - **Why:** The spec must reflect the actual supported targets. - **Verify:** The code block contains only `net10.0;net9.0;net8.0`. -- [ ] **`spec/tech-standards/build-system.md` line 33** — Delete the `| \`netstandard2.0\` | Broad compatibility (.NET Framework 4.6.1+, .NET Core 2.0+) |` row from the target table. +- [x] **`spec/tech-standards/build-system.md` line 33** — Delete the `| \`netstandard2.0\` | Broad compatibility (.NET Framework 4.6.1+, .NET Core 2.0+) |` row from the target table. - **Why:** The target no longer exists; the row is misleading. - **Verify:** No mention of `netstandard2.0` remains anywhere in `build-system.md`. diff --git a/spec/tech-standards/build-system.md b/spec/tech-standards/build-system.md index 0792541..4a72e25 100644 --- a/spec/tech-standards/build-system.md +++ b/spec/tech-standards/build-system.md @@ -22,7 +22,7 @@ HdrHistogram.sln ### Main Library (HdrHistogram.csproj) ```xml -net10.0;net9.0;net8.0;netstandard2.0 +net10.0;net9.0;net8.0 ``` | Target | Description | @@ -30,7 +30,6 @@ HdrHistogram.sln | `net10.0` | Modern .NET (current LTS target) | | `net9.0` | Modern .NET (STS target) | | `net8.0` | Modern .NET (LTS target) | -| `netstandard2.0` | Broad compatibility (.NET Framework 4.6.1+, .NET Core 2.0+) | ### Test Project From f97a0dc867186965f3947b4b5b73dde7913fe42b Mon Sep 17 00:00:00 2001 From: leecampbell-codeagent Date: Mon, 23 Mar 2026 01:41:22 +0000 Subject: [PATCH 7/7] feat(#161): complete implementation --- plan/done/brief.md | 76 ---------------------------------------------- plan/done/task.md | 67 ---------------------------------------- 2 files changed, 143 deletions(-) delete mode 100644 plan/done/brief.md delete mode 100644 plan/done/task.md diff --git a/plan/done/brief.md b/plan/done/brief.md deleted file mode 100644 index 7737b4f..0000000 --- a/plan/done/brief.md +++ /dev/null @@ -1,76 +0,0 @@ -# Brief: Issue #161 — Drop netstandard2.0 target, support net8.0+ only - -## Summary - -Remove `netstandard2.0` from the main library's ``, retaining only `net10.0;net9.0;net8.0`. -This eliminates all conditional compilation guards that existed solely to support the older target, simplifies `Bitwise.cs` to use `BitOperations.LeadingZeroCount` unconditionally, and cleans up a branching code path in `HistogramLogReader.cs`. -The three per-framework Release `PropertyGroup` conditions in the csproj are collapsed into a single unconditional `PropertyGroup` as part of this change. -The spec file `build-system.md` must be updated to reflect the new target list. - -Previous NuGet releases remain available on NuGet.org for consumers on .NET Framework or older runtimes. - -## Category - -`chore` - -This is a maintenance and simplification task. -The hardware-intrinsic `LeadingZeroCount` path (`#if NET5_0_OR_GREATER`) is already compiled in for all net8.0+ builds; removing the fallback does not change the compiled output for any net8.0, net9.0, or net10.0 consumer. -No performance improvement is delivered to existing net8.0+ consumers — the change reduces build artefacts, removes dead code, and simplifies the codebase. - -## Affected Files (confirmed by exploration) - -| File | Change required | -|------|----------------| -| `HdrHistogram/HdrHistogram.csproj` | Remove `netstandard2.0` from `` (line 4); delete the netstandard2.0 `PropertyGroup` condition (lines 36-39); collapse the three per-framework Release `PropertyGroup` conditions (lines 24-34) into a single `Condition="'$(Configuration)' == 'Release'"` block | -| `HdrHistogram/Utilities/Bitwise.cs` | Remove `#if NET5_0_OR_GREATER` guards (lines 25-29, 32-44); inline `IntrinsicNumberOfLeadingZeros` body directly into `NumberOfLeadingZeros`; delete the entire `Bitwise.Imperative` class (lines 55-109) | -| `HdrHistogram/HistogramLogReader.cs` | Remove `#if NETSTANDARD2_0` block in `IsComment()` (lines 241-248); keep the `char` overload unconditionally | -| `spec/tech-standards/build-system.md` | Remove `netstandard2.0` from the TargetFrameworks code block (line 25) and from the target table (line 33) | -| `HdrHistogram.Benchmarking/LeadingZeroCount/LeadingZeroCountBenchmarkBase.cs` | Remove the `ImperativeImplementation` benchmark method (references deleted `Bitwise.Imperative` class) | - -No other files in the codebase contain `#if NETSTANDARD` or `#if NET5_0_OR_GREATER` blocks. -Unit-test and benchmarking projects already target `net10.0;net9.0;net8.0` only — no changes needed there. - -## Acceptance Criteria - -- [ ] Library targets `net10.0;net9.0;net8.0` only; `netstandard2.0` is absent from `HdrHistogram.csproj` -- [ ] No `#if NETSTANDARD` or `#if NET5_0_OR_GREATER` conditional compilation remains anywhere in the library -- [ ] `Bitwise.Imperative` class is fully removed -- [ ] `Bitwise.NumberOfLeadingZeros` calls `System.Numerics.BitOperations.LeadingZeroCount` unconditionally -- [ ] `HistogramLogReader.IsComment` uses `line.StartsWith('#')` (char overload) unconditionally -- [ ] The three per-framework Release `PropertyGroup` conditions are collapsed into a single `Condition="'$(Configuration)' == 'Release'"` block -- [ ] `LeadingZeroCountBenchmarkBase.cs` no longer references `Bitwise.Imperative`; the benchmarking project compiles cleanly -- [ ] All unit tests pass on net8.0, net9.0, and net10.0 -- [ ] `spec/tech-standards/build-system.md` no longer references `netstandard2.0` - -## Test Strategy - -### Existing tests - -No unit tests directly target `Bitwise` or `HistogramLogReader.IsComment` in isolation. -The existing suite exercises both paths indirectly via histogram recording and log-reading tests. -Run the full unit-test suite across all three target frameworks after the changes: - -``` -dotnet test -c Release -``` - -All tests must pass; no new test failures are acceptable. - -### New tests (optional but recommended) - -Consider adding a focused unit test in `HdrHistogram.UnitTests/` that asserts `Bitwise.NumberOfLeadingZeros` returns correct values for a range of inputs (including 0, 1, powers-of-two, and `long.MaxValue`). -This is low-risk but provides a regression anchor if `Bitwise.cs` is touched again. - -There is no need to add tests for `IsComment` — it is a private static helper with trivial logic. - -## Risks and Open Questions - -1. **Benchmark class references `Bitwise.Imperative`** — deleting the class will break the `ImperativeImplementation` benchmark at compile time. - The benchmark must be updated in the same PR (listed in Affected Files above). - -2. **NuGet package surface** — removing a target framework is a breaking change for consumers on netstandard2.0. - This is explicitly accepted: "Previous NuGet releases will remain available." - The PR description should note this prominently and suggest a major-version bump or release notes entry. - -3. **No other conditional compilation found** — the grep over the codebase confirmed only two files contain the relevant `#if` blocks. - Low risk of missing anything. diff --git a/plan/done/task.md b/plan/done/task.md deleted file mode 100644 index 7079ea3..0000000 --- a/plan/done/task.md +++ /dev/null @@ -1,67 +0,0 @@ -# Task List: Issue #161 — Drop netstandard2.0 target, support net8.0+ only - -## Implementation Changes - -- [x] **`HdrHistogram/HdrHistogram.csproj` line 4** — Remove `netstandard2.0` from ``, leaving `net10.0;net9.0;net8.0`. - - **Why:** Drops the legacy target entirely. - - **Verify:** `` value contains no `netstandard2.0` token. - -- [x] **`HdrHistogram/HdrHistogram.csproj` lines 24–34** — Collapse the three per-framework Release `PropertyGroup` blocks (one each for `net8.0`, `net9.0`, `net10.0`) into a single `PropertyGroup Condition="'$(Configuration)' == 'Release'"` block with a single `` element that resolves via `$(TargetFramework)`. - - **Why:** Eliminates duplicated XML and the framework-specific condition strings. - - **Verify:** Only one Release `PropertyGroup` exists; it contains no `$(TargetFramework)` literals in the condition string. - -- [x] **`HdrHistogram/HdrHistogram.csproj` lines 36–39** — Delete the `netstandard2.0` `PropertyGroup` block (the one setting `DefineConstants` to `RELEASE;NETSTANDARD2_0`). - - **Why:** The constant `NETSTANDARD2_0` is no longer needed once the target is removed. - - **Verify:** No `PropertyGroup` referencing `netstandard2.0` or `NETSTANDARD2_0` remains in the file. - -- [x] **`HdrHistogram/Utilities/Bitwise.cs` lines 23–44** — Simplify `NumberOfLeadingZeros(long)` to call `System.Numerics.BitOperations.LeadingZeroCount((ulong)value)` directly; remove the `#if NET5_0_OR_GREATER` / `#else` / `#endif` guards and delete the private `IntrinsicNumberOfLeadingZeros` helper. - - **Why:** All supported targets (net8.0+) provide `BitOperations.LeadingZeroCount`; the conditional dispatch is dead code. - - **Verify:** `NumberOfLeadingZeros` body is a single `return System.Numerics.BitOperations.LeadingZeroCount((ulong)value);` statement; no `#if` directives remain in the method or immediately around it. - -- [x] **`HdrHistogram/Utilities/Bitwise.cs` lines 55–109** — Delete the entire `Bitwise.Imperative` nested public static class (including the `Lookup` table, `NumberOfLeadingZeros`, `NumberOfLeadingZerosLong`, and `Log2` methods). - - **Why:** The imperative fallback path is unreachable on net8.0+; removing it eliminates dead code and the public surface that the benchmark references. - - **Verify:** No `class Imperative` or `Bitwise.Imperative` identifier exists anywhere in the solution. - -- [x] **`HdrHistogram/HistogramLogReader.cs` lines 241–248** — Remove the `#if NETSTANDARD2_0` / `#else` / `#endif` block inside `IsComment(string line)`, keeping only `return line.StartsWith('#');`. - - **Why:** The `char` overload of `StartsWith` is available on all net8.0+ targets; the string-overload fallback is dead code. - - **Verify:** `IsComment` contains no `#if` directives; the method body is `return line.StartsWith('#');`. - -- [x] **`HdrHistogram.Benchmarking/LeadingZeroCount/LeadingZeroCountBenchmarkBase.cs`** — Remove the `"Imperative"` entry from the validation dictionary (line 56) and delete the `ImperativeImplementation()` benchmark method (lines 124–133). - - **Why:** Both reference `Bitwise.Imperative` which will no longer exist; leaving them causes a compile error. - - **Verify:** `dotnet build HdrHistogram.Benchmarking/ -c Release` exits with code 0; no reference to `Bitwise.Imperative` remains in the file. - -## Unit Tests - -- [x] **`HdrHistogram.UnitTests/`** — Add a focused unit test class `BitwiseTests` (e.g. `HdrHistogram.UnitTests/Utilities/BitwiseTests.cs`) that asserts `Bitwise.NumberOfLeadingZeros` returns correct results for representative inputs: `0`, `1`, `2`, powers of two up to 2⁶², and `long.MaxValue`. - - **Why:** No existing test directly covers `Bitwise`; this provides a regression anchor if the method is ever touched again. - - **Verify:** Test class exists; `dotnet test -c Release` reports the new tests as passing on all three target frameworks. - -- [x] **Run the full unit-test suite** — Execute `dotnet test -c Release` across all three target frameworks (net8.0, net9.0, net10.0). - - **Why:** Confirms that removing the netstandard2.0 conditional paths has not broken any indirect consumer of `Bitwise` or `HistogramLogReader`. - - **Verify:** Zero test failures; zero skipped tests that were previously passing. - -## Documentation - -- [x] **`spec/tech-standards/build-system.md` line 25** — Remove `netstandard2.0` from the `` code block example. - - **Why:** The spec must reflect the actual supported targets. - - **Verify:** The code block contains only `net10.0;net9.0;net8.0`. - -- [x] **`spec/tech-standards/build-system.md` line 33** — Delete the `| \`netstandard2.0\` | Broad compatibility (.NET Framework 4.6.1+, .NET Core 2.0+) |` row from the target table. - - **Why:** The target no longer exists; the row is misleading. - - **Verify:** No mention of `netstandard2.0` remains anywhere in `build-system.md`. - ---- - -## Acceptance Criteria Cross-Reference - -| Acceptance criterion (from brief) | Covered by task | -|---|---| -| Library targets `net10.0;net9.0;net8.0` only; `netstandard2.0` absent from `.csproj` | Task 1 (TargetFrameworks) | -| No `#if NETSTANDARD` or `#if NET5_0_OR_GREATER` conditional compilation in library | Tasks 4, 6 (Bitwise.cs, HistogramLogReader.cs) | -| `Bitwise.Imperative` class fully removed | Task 5 | -| `Bitwise.NumberOfLeadingZeros` calls `BitOperations.LeadingZeroCount` unconditionally | Task 4 | -| `HistogramLogReader.IsComment` uses `line.StartsWith('#')` unconditionally | Task 6 | -| Three per-framework Release `PropertyGroup` conditions collapsed into one | Task 2 | -| `LeadingZeroCountBenchmarkBase.cs` no longer references `Bitwise.Imperative`; benchmarking project compiles | Task 7 | -| All unit tests pass on net8.0, net9.0, net10.0 | Tasks 8 (new tests), 9 (full suite) | -| `spec/tech-standards/build-system.md` no longer references `netstandard2.0` | Tasks 10, 11 |