Skip to content

Conversation

@peter-lawrey
Copy link
Member

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

This PR improves developer ergonomics and hardens the Thread Affinity codebase:

  • Adds an opt-in code-review Maven profile (Checkstyle, SpotBugs+FindSecBugs, PMD, JaCoCo) to both affinity and affinity-test.
  • Expands documentation and readability in README.adoc, enables :sectnums:, and clarifies examples.
  • Normalises LICENSE.adoc line-wrapping (no content change).
  • Introduces portable JNA stubs (opt-in via system properties) so the test suite can run on non-native OSes.
  • Fixes minor logging and style issues; tightens charset handling; removes a few footguns.
  • Adds focused unit tests for masks, stubs, logging, timers, and sample app behaviour.

Motivation

  • Provide a repeatable quality gate developers can run locally and in CI without changing default builds.
  • Make docs easier to scan, copy, and keep in sync with policy (British English, AsciiDoc structure).
  • Improve portability and determinism of tests, especially on CI runners not matching target OS.
  • Reduce subtle risks around encodings, logging of errors, and concurrency patterns.

What changed

Build & Quality

  • affinity and affinity-test POMs:

    • New properties for tool versions and coverage thresholds.

    • New code-review profile wiring:

      • Checkstyle using net.openhft:chronicle-quality-rules and com.puppycrawl.tools:checkstyle.
      • SpotBugs + FindSecBugs with project-local exclude filters.
      • PMD with project-local ruleset/exclusions.
      • JaCoCo with bundle-level coverage gates (module-specific thresholds).
  • Minor XML formatting normalisation (relativePath/ empty-element, multi-line project header).

Documentation

  • LICENSE.adoc: collapsed long lines; no legal text change.

  • README.adoc:

    • :sectnums: enabled.
    • Small clarity edits; adds example for large core indices in affinity.reserved (CPU 64).
    • Fixes fenced blocks and minor whitespace alignment.
  • affinity/src/main/adoc/requirements.adoc:

    • Adds :pp: ++ and :sectnums:.
    • Harmonises section numbering and C/C++ references (written as C/C{pp}).

Source changes (highlights)

  • Affinity, AffinityLock, AffinityThreadFactory, BootClassPath, LockCheck:

    • Minor logging/formatting tidy-ups, safer debug logging, and small style improvements.
  • FileLockBasedLockChecker:

    • All file I/O now uses UTF-8 explicitly; better handling of tmpdir creation failure.
  • Utilities:

    • UTF-8 explicit; try-with-resources for writers.
  • MicroJitterSampler / JNIClock:

    • Replace empty spin-loops with clearly marked busy-wait blocks (//NOPMD) and comments.
  • LinuxHelper / PosixJNAAffinity / OSXJNAAffinity / SolarisJNAAffinity / WindowsJNAAffinity:

    • Introduce test stubs for JNA back-ends, gated by system properties:

      • chronicle.affinity.stub.linux
      • chronicle.affinity.stub.posix
      • chronicle.affinity.stub.osx
      • chronicle.affinity.stub.solaris
      • chronicle.affinity.stub.windows
    • Linux helper gains robust version probing, UTF-8 decoding, and macro-aligned cpu_set_t sizing.

    • Windows implementation wraps JNA loading in a container, with a stub fallback and defensive calls to Kernel32.

    • POSIX and Linux paths use a new CpuSetUtil for mask sizing/encoding beyond 64 CPUs.

  • Annotations:

    • Add targeted @SuppressFBWarnings and adjust method modifiers where appropriate (e.g., BootClassPath.has).

New configuration files

  • affinity/src/main/config/spotbugs-exclude.xml
  • affinity/src/main/config/pmd-exclude.properties
  • affinity-test/src/main/config/pmd-exclude.properties

Tests

New and updated tests to cover:

  • Mask math & round-trips beyond 64 CPUs (CpuSetUtilTest, PosixAffinityMaskTest, LinuxHelperCpuSetTest).

  • JNA stubbed modes across OS variants:

    • LinuxJNAAffinityStubTest, PosixJNAAffinityStubTest, OSXJNAAffinityStubTest,
    • SolarisJNAAffinityStubTest, WindowsJNAAffinityStubTest.
  • Lock acquisition edge cases, inventory behaviour, and logging (AffinityLockInvalidCpuTest, LockInventoryLoggingTest).

  • Sample app lifecycle and interrupt handling (AffinityTestMainTest) with injectable lock supplier.

  • Ticker/clock selection and conversions (TickerTest).

  • Minor README code fences and usage examples validated via existing integration tests.

How to use

  • Normal build (unchanged):
mvn -q verify
  • Run the code-review gate locally (opt-in):
mvn -q -Pcode-review verify
  • Enable a JNA stub to exercise tests on non-native OS (example):
# Choose one as appropriate for the platform you are *not* on:
mvn -q -Pcode-review -Dchronicle.affinity.stub.linux=true verify

Compatibility

  • API/ABI: No public API removals. Minor visibility relaxation (BootClassPath.has) and safe refactors only.
  • Behaviour: No functional change intended on supported native paths. Hashing/timing behaviour unchanged.
  • Windows detection: When running in stub mode, WindowsJNAAffinity.LOADED remains false by design.

Risks & mitigations

  • Introducing stubs could mask native regressions if left enabled. Mitigation: stubs are opt-in via explicit system properties; CI can include native lanes.
  • Expanded static analysis might flag existing legacy patterns. Mitigation: scoped, documented exclusions with follow-up tags (e.g., AFF-SEC-310, AFF-OPS-330).

@peter-lawrey peter-lawrey changed the title Adv/code review Add code-review profile, richer docs, safer I/O; introduce JNA stubs and new tests Oct 28, 2025
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