-
Notifications
You must be signed in to change notification settings - Fork 8
Tighten JLBH encoding policy, constructor safety, and documentation traceability #176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
peter-lawrey
wants to merge
14
commits into
develop
Choose a base branch
from
adv/develop
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 4 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
fb1048f
Enhance documentation with section numbering and improve formatting a…
peter-lawrey fc367db
Refactor JLBH classes to improve thread safety and code clarity
peter-lawrey 1219bfb
Refactor JLBH classes for improved UTF-8 support and code clarity
peter-lawrey 7db0935
Add CLAUDE.md for project guidance and documentation structure
peter-lawrey 3fdfc8f
Add documentation for JLBH system properties
peter-lawrey c457b53
Updated documentation
peter-lawrey 443b978
Enhance documentation with British English conventions and formatting…
peter-lawrey 93e18d3
Update documentation
peter-lawrey b8cd0f2
Add SpotBugs profile and enhance development guidelines
peter-lawrey a18896e
Checkpoint
peter-lawrey 56ea0d3
Update TODO.md with ISO alignment and trust zone details for multiple…
peter-lawrey 17a449e
Code Analysis fixes
peter-lawrey 23cc178
Remove unnecessary blank lines in various classes for improved code r…
peter-lawrey 84d5d1d
Remove unnecessary blank lines in various classes for improved code r…
peter-lawrey File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,219 @@ | ||
| # CLAUDE.md | ||
|
|
||
| This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. | ||
|
|
||
| ## Project Overview | ||
|
|
||
| JLBH (Java Latency Benchmark Harness) is a specialised benchmarking library for measuring end-to-end latency in Java applications under realistic throughput conditions. Unlike JMH micro-benchmarks, JLBH is designed to benchmark code "in context" and account for coordinated omission. It is particularly suited for producer/consumer workloads where iteration start and completion may occur on different threads. | ||
|
|
||
| ## Build & Test Commands | ||
|
|
||
| ```bash | ||
| # Verify build and run all tests | ||
| mvn -q verify | ||
|
|
||
| # Clean build from scratch | ||
| mvn -q clean verify | ||
|
|
||
| # Run a specific test class | ||
| mvn -q test -Dtest=JLBHTest | ||
|
|
||
| # Run a single test method | ||
| mvn -q test -Dtest=JLBHTest#shouldProvideResultData | ||
|
|
||
| # Compile only (skip tests) | ||
| mvn -q compile | ||
|
|
||
| # Skip tests during verify | ||
| mvn -q verify -DskipTests | ||
| ``` | ||
|
|
||
| ## Language & Character-Set Requirements | ||
|
|
||
| **Critical**: All code and documentation must use: | ||
| - **British English** spelling (`organisation`, `licence`) except for Java keywords like `synchronized` | ||
| - **ISO-8859-1** character encoding only (code points 0-255) | ||
| - **No Unicode characters**: Use ASCII equivalents like `>=` instead of special symbols | ||
| - Verify with: `iconv -f ascii -t ascii <file>` | ||
|
|
||
| See AGENTS.md for complete language policy and rationale. | ||
|
|
||
| ## Architecture Overview | ||
|
|
||
| ### Core Components | ||
|
|
||
| **JLBH** (src/main/java/net/openhft/chronicle/jlbh/JLBH.java) | ||
| - The main orchestrator that manages the entire benchmark lifecycle | ||
| - Runs on a single thread (annotated `@SingleThreaded`) | ||
| - Provides `sample(long durationNs)` to record end-to-end latencies | ||
| - Provides `addProbe(String)` to create additional `NanoSampler` instances for measuring sub-stages | ||
| - Can run standalone via `start()` or integrate with Chronicle EventLoop via `eventLoopHandler(EventLoop)` | ||
|
|
||
| **JLBHOptions** (src/main/java/net/openhft/chronicle/jlbh/JLBHOptions.java) | ||
| - Builder-style configuration object defining all benchmark parameters | ||
| - Key settings: `throughput()`, `iterations()`, `runs()`, `warmUpIterations()`, `accountForCoordinatedOmission()`, `recordOSJitter()` | ||
| - Default throughput: 10,000 ops/sec; default runs: 3 | ||
|
|
||
| **JLBHTask** (src/main/java/net/openhft/chronicle/jlbh/JLBHTask.java) | ||
| - User-implemented interface defining the workload to benchmark | ||
| - Lifecycle methods called by JLBH: | ||
| - `init(JLBH)`: Setup phase, register probes | ||
| - `run(long startTimeNs)`: Execute benchmark iteration (called for each iteration) | ||
| - `warmedUp()`: Notification after warmup completes | ||
| - `runComplete()`: Notification after each run | ||
| - `complete()`: Final cleanup | ||
|
|
||
| **Histograms & Results** | ||
| - Each probe (end-to-end, custom probes, OS jitter) records samples into a `Histogram` from chronicle-core | ||
| - Histograms use logarithmic bucketing with 35-bit range and 8 significant figures | ||
| - Immutable result objects: `JLBHResult`, `ProbeResult`, `RunResult` | ||
| - Use `ThreadSafeJLBHResultConsumer` to retrieve results from another thread | ||
|
|
||
| ### Threading Model | ||
|
|
||
| - **Single-threaded harness**: All JLBHTask lifecycle methods run on the single JLBH thread | ||
| - **User code may spawn threads**: The benchmarked code within `JLBHTask.run()` can be multi-threaded | ||
| - **Sample recording thread-safety**: Only the JLBH harness thread should call `jlbh.sample()` or probe samplers | ||
| - **Event loop integration**: Via `eventLoopHandler()` instead of `start()` (requires coordinated omission accounting enabled) | ||
|
|
||
| ### Execution Flow | ||
|
|
||
| 1. **Initialization**: User configures `JLBHOptions`, creates `JLBH` instance, calls `JLBHTask.init()` | ||
| 2. **Warm-up**: Runs `warmUpIterations` times to allow JIT compilation (default ~12k iterations) | ||
| 3. **Measurement runs**: Executes `runs` times (default 3), each with `iterations` samples | ||
| 4. **Completion**: Prints summary, constructs immutable `JLBHResult`, calls `JLBHTask.complete()` | ||
|
|
||
| See src/main/adoc/benchmark-lifecycle.adoc for visual flow diagram. | ||
|
|
||
| ### Coordinated Omission | ||
|
|
||
| JLBH accounts for coordinated omission by default (`accountForCoordinatedOmission = true`): | ||
| - `startTimeNs` passed to `run()` is the *calculated ideal start time*, not `System.nanoTime()` | ||
| - Harness busy-waits if current time is before scheduled start time | ||
| - Disabling this feature means start time is simply based on throughput without waiting | ||
|
|
||
| ### OS Jitter Monitoring | ||
|
|
||
| Enabled by default via `recordOSJitter(true)`: | ||
| - Background thread `OSJitterMonitor` repeatedly samples `System.nanoTime()` to detect scheduler delays | ||
| - Records jitter > `recordJitterGreaterThanNs` (default: 1000ns) into separate histogram | ||
| - Incurs overhead; disable with `recordOSJitter(false)` for minimal-overhead benchmarks | ||
|
|
||
| ## Key Design Patterns | ||
|
|
||
| **Probes for Multi-Stage Benchmarks** | ||
| ```java | ||
| class MyTask implements JLBHTask { | ||
| private JLBH jlbh; | ||
| private NanoSampler stage1Probe; | ||
|
|
||
| public void init(JLBH jlbh) { | ||
| this.jlbh = jlbh; | ||
| this.stage1Probe = jlbh.addProbe("stage1"); | ||
| } | ||
|
|
||
| public void run(long startTimeNs) { | ||
| long stage1Start = System.nanoTime(); | ||
| // ... stage 1 work ... | ||
| stage1Probe.sampleNanos(System.nanoTime() - stage1Start); | ||
|
|
||
| // ... remaining work ... | ||
| jlbh.sample(System.nanoTime() - startTimeNs); | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| **Typical Benchmark Setup** | ||
| ```java | ||
| JLBHOptions options = new JLBHOptions() | ||
| .warmUpIterations(100_000) | ||
| .iterations(1_000_000) | ||
| .throughput(50_000) | ||
| .runs(3) | ||
| .jlbhTask(myTask); | ||
| new JLBH(options).start(); | ||
| ``` | ||
|
|
||
| **Thread-Safe Result Retrieval** | ||
| ```java | ||
| JLBHResultConsumer resultConsumer = JLBHResultConsumer.newThreadSafeInstance(); | ||
| JLBH jlbh = new JLBH(options, System.out, resultConsumer); | ||
| // Run on separate thread | ||
| new Thread(() -> jlbh.start()).start(); | ||
| // Retrieve results from main thread after completion | ||
| JLBHResult result = resultConsumer.get(); | ||
| ``` | ||
|
|
||
| ## Documentation Structure | ||
|
|
||
| All requirements and design decisions are in src/main/adoc/: | ||
| - **project-requirements.adoc**: Formal requirements specification with tagged requirements (FN-xxx, NF-P-xxx, etc.) | ||
| - **decision-log.adoc**: Architectural decision records (ADRs) following Nine-Box taxonomy | ||
| - **architecture.adoc**: High-level architecture, components, execution flow, threading model | ||
| - **benchmark-lifecycle.adoc**: Visual diagram of benchmark execution phases | ||
| - **jlbh-cookbook.adoc**: Common usage patterns and recipes | ||
| - **results-interpretation-guide.adoc**: How to interpret percentile output | ||
|
|
||
| When making changes: | ||
| 1. Update relevant .adoc files first (if changing requirements/design) | ||
| 2. Update code and tests | ||
| 3. Verify all three stay synchronised in same commit | ||
|
|
||
| ## Javadoc Standards | ||
|
|
||
| Follow guidelines from AGENTS.md: | ||
| - Document *behavioural contracts*, edge cases, thread-safety, units, performance characteristics | ||
| - Do NOT restate the obvious ("Gets the value") | ||
| - First sentence must be concise (becomes summary line) | ||
| - Remove autogenerated Javadoc for trivial getters/setters | ||
| - Explain *why* and *how*, not just *what* | ||
|
|
||
| ## Test Examples | ||
|
|
||
| Test files in src/test/java/net/openhft/chronicle/jlbh/ demonstrate usage: | ||
| - `ExampleJLBHMain`: Basic command-line harness demonstration | ||
| - `SimpleBenchmark`: Minimal benchmark example | ||
| - `SimpleOSJitterBenchmark`: Demonstrates OS jitter recording and custom probes | ||
| - `JLBHTest`: Unit test showing programmatic result extraction | ||
| - `JLBHIntegrationTest`: Example integration test | ||
|
|
||
| ## Performance Requirements | ||
|
|
||
| From project-requirements.adoc (NF-P requirements): | ||
| - Overhead per sample: < 100 ns when no additional probes active | ||
| - Histogram generation: Support >= 200M iterations without heap pressure | ||
| - Warm-up rule of thumb: ~30% of run iterations (default uses JIT compile threshold * 6/5) | ||
|
|
||
| ## Dependencies | ||
|
|
||
| Key dependencies from pom.xml: | ||
| - **chronicle-core**: Provides `Histogram`, `NanoSampler`, threading utilities | ||
| - **affinity**: CPU affinity control via `AffinityLock` | ||
| - **chronicle-threads**: Event loop integration (test scope) | ||
|
|
||
| ## Common Gotchas | ||
|
|
||
| - JLBH is `@SingleThreaded` - the harness itself runs on one thread | ||
| - The `startTimeNs` parameter to `run()` is NOT `System.nanoTime()` - it's the calculated ideal start time | ||
| - Warm-up iterations use ~12k by default; adjust with `warmUpIterations()` if benchmark takes long to stabilise | ||
| - OS jitter monitoring is enabled by default and adds overhead; explicitly disable if needed | ||
| - When using `eventLoopHandler()`, coordinated omission must remain enabled | ||
| - CSV export: Use `JLBHResultSerializer.runResultToCSV(jlbhResult)` (writes to `result.csv` by default) | ||
|
|
||
| ## Non-Functional Quality Attributes | ||
|
|
||
| From README.adoc summary: | ||
| - **Performance**: < 100 ns overhead per sample, >= 200M iterations supported | ||
| - **Reliability**: Graceful abort on interruption/timeout, immutable thread-safe results | ||
| - **Usability**: Fluent API, runnable in <= 10 LOC, human-readable ASCII output | ||
| - **Portability**: Pure Java, runtime-detected JDK optimisations | ||
| - **Maintainability**: >= 80% test coverage (current: ~83% line, ~78% branch per pom.xml) | ||
|
|
||
| ## Commit & PR Guidelines | ||
|
|
||
| From AGENTS.md: | ||
| - Subject line <= 72 chars, imperative mood | ||
| - Body: root cause -> fix -> measurable impact | ||
| - Run `mvn -q verify` before opening PR | ||
| - Keep PRs focused; avoid bundling unrelated changes | ||
| - Re-run build after addressing review comments |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just needs updating to point to latest BOM