Skip to content

Conversation

@peter-lawrey
Copy link
Member

@peter-lawrey peter-lawrey commented Oct 26, 2025

This PR introduces a shared code-review profile that runs Chronicle’s quality toolchain (Checkstyle, PMD, SpotBugs/FindSecBugs, JaCoCo) and adds focused tests to lift coverage, particularly around POSIX provider selection, affinity-mask sizing, and JNA wrappers. Documentation is normalised with numbered sections to improve navigability.

Why

  • Establish a consistent, opt-in quality gate developers can run locally and in CI.
  • Catch portability issues and API regressions early (affinity masks, provider fallbacks).
  • Increase confidence via targeted unit tests for previously under-tested paths.

What changed

Docs

  • README.adoc, decision-log.adoc, project-requirements.adoc: enable :sectnums: and minor cleanups.

Production code

  • PosixAPI.java:

    • Clarify and tighten Javadoc; prefer concise, API-first descriptions.

    • Fix affinity mask sizing and access:

      • Introduce requiredMaskBytes(int) to size masks by CPU index, rounded to Long.BYTES.
      • Use word/bit addressing that scales beyond 32 CPUs.
    • Safer sched_setaffinity_range(...) with argument validation.

    • Simplify JNA NULL handling in JNAPosixAPI via Pointer.NULL.

  • PosixAPIHolder.java:

    • Eager runtime probe (getpid()) to force early fallback to NoOpPosixAPI when native loading fails.
    • Comment updates and explicit no-op switch wording.

Tests (new)

  • PosixAffinityHelperTest: verifies mask sizing beyond 64 CPUs, range checks, and bit placement.
  • PosixAPIHolderTest: enforces provider order and confirms No-Op fallback when native runtime is unavailable.
  • JNAPosixAPITest: confirms mmap wrapper passes Pointer.NULL for zero address and non-zero pointer otherwise.
  • NoOpPosixAPITest: documents and asserts the no-op vs throwing behaviour surface of the fallback provider.
  • RawPosixAPITest: asserts default memory-lock helpers remain no-ops/false in raw provider subclasses.

How to run quality checks

  • Local: mvn -Pcode-review -q verify
  • CI: enable the code-review profile in the workflow to gate on Checkstyle, PMD, SpotBugs, and JaCoCo.

Coverage impact

  • New tests exercise provider bootstrapping, affinity helpers, and JNA bridging paths that were previously untested.
  • Expect an increase in both line and branch coverage, especially under net.openhft.posix.*.

Backwards compatibility

  • No breaking API changes.

  • Behavioural fixes:

    • Correct CPU mask sizing prevents truncation on hosts with high core counts.
    • Earlier fallback to NoOpPosixAPI yields clearer failure modes on misconfigured systems.

Risks and mitigations

  • Low: changes are mostly additive or corrective, with tests capturing intended behaviour.
  • Rollback: revert this PR to restore prior mask handling and provider load semantics.

Follow-ups

  • Consider lifting common suppressions and helper utilities into the shared Chronicle ruleset.
  • Extend tests to cover Windows provider behaviour in a CI matrix when feasible.

@peter-lawrey peter-lawrey changed the title Adv/code review Harden CPU-affinity handling; fix cpuset overrun; document policy; add regression tests Oct 26, 2025
@peter-lawrey peter-lawrey changed the title Harden CPU-affinity handling; fix cpuset overrun; document policy; add regression tests Add code-review profile, wire quality checks, and raise test coverage Oct 27, 2025
@peter-lawrey peter-lawrey marked this pull request as draft November 3, 2025 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants