Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 153 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ description = """
`mdtablefix` unb0rks and reflows Markdown tables so that each column has a uniform width. When \
the `--wrap` option is used, it also wraps paragraphs and list items to 80 columns."""

[features]
# Exposes internal inline-wrapping shims and fixtures used only by
# `benches/wrap_observer.rs`. Never enable in production; it is not part of the
# public API and only affects `#[doc(hidden)]` benchmark support code.
bench-internals = []
[package.metadata.binstall]

[package.metadata.binstall.overrides.'cfg(all(target_os = "linux", any(target_arch = "x86_64", target_arch = "aarch64"), target_env = "gnu"))']
Expand Down Expand Up @@ -44,6 +49,11 @@ predicates = "3"
trybuild = "1"
tracing-test = "0.2"
test-macros = { path = "test-macros" }
criterion = { version = "0.5", default-features = false, features = ["cargo_bench_support"] }

[[bench]]
name = "wrap_observer"
harness = false
required-features = ["bench-internals"]
[lints.clippy]
pedantic = "warn"
5 changes: 4 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.PHONY: help all clean test build release lint typecheck fmt check-fmt check-ripgrep check-static-regexes markdownlint nixie
.PHONY: help all clean test bench build release lint typecheck fmt check-fmt check-ripgrep check-static-regexes markdownlint nixie

APP ?= mdtablefix
CARGO ?= $(or $(shell command -v cargo 2>/dev/null),$(HOME)/.cargo/bin/cargo)
Expand All @@ -19,6 +19,9 @@ clean: ## Remove build artifacts
test: ## Run tests with warnings treated as errors
RUSTFLAGS="-D warnings" $(CARGO) test --all-targets --all-features $(BUILD_JOBS)

bench: ## Run the observer-boundary wrapping benchmarks
$(CARGO) bench --features bench-internals --bench wrap_observer

Comment on lines +22 to +24

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Make bench fail on warnings.

This target invokes cargo bench without -D warnings, so benchmark-only warnings can pass and the stated warnings-as-errors validation requirement is not enforced. Propagate RUSTFLAGS with -D warnings, or enforce the equivalent in CI.

Proposed fix
 bench: ## Run the observer-boundary wrapping benchmarks
-	$(CARGO) bench --features bench-internals --bench wrap_observer
+	RUSTFLAGS="$(RUSTFLAGS) -D warnings" \
+	$(CARGO) bench --features bench-internals --bench wrap_observer
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Makefile` around lines 22 - 24, Update the Makefile bench target’s cargo
invocation to propagate the existing RUSTFLAGS with -D warnings, ensuring
benchmark compilation fails on warnings while preserving the current features
and wrap_observer benchmark selection.

target/%/$(APP): ## Build binary in debug or release mode
$(CARGO) build $(BUILD_JOBS) $(if $(findstring release,$(@)),--release) --bin $(APP)

Expand Down
57 changes: 57 additions & 0 deletions benches/wrap_observer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
//! Benchmarks for the inline-wrapping observer boundary.
//!
//! These benchmarks protect the performance invariant introduced with the
//! observer/tracing adapter (see `docs/adrs/0006-observer-boundary-for-tracing.md`):
//! with tracing disabled, `TracingObserver` must add no derived-payload work —
//! Unicode length counts or snippet truncation — beyond a single
//! `tracing::enabled!` branch per event. Comparing the `observer_none` and
//! `tracing_observer_disabled` inline cases makes any regression in that
//! invariant visible as a growing gap between the two.
//!
//! Run with:
//!
//! ```sh
//! cargo bench --features bench-internals --bench wrap_observer
//! ```

use std::hint::black_box;

use criterion::{Criterion, criterion_group, criterion_main};
use mdtablefix::{
wrap::bench_internals::{
BENCH_WIDTH,
large_inline_paragraph,
realistic_markdown_document,
wrap_with_tracing_observer,
wrap_without_observer,
},
wrap_text,
};

/// Benchmarks the public `wrap_text` path over a large realistic document.
///
/// `wrap_text` always drives the wrapping pipeline through `TracingObserver`,
/// so this measures the library boundary with no subscriber installed.
fn bench_public_document(c: &mut Criterion) {
let document = realistic_markdown_document();
c.bench_function("wrap_text_realistic_document", |b| {
b.iter(|| wrap_text(black_box(&document), black_box(BENCH_WIDTH)));
});
}

/// Benchmarks the inline hot path with the observer disabled (`None`) and with
/// `TracingObserver` while no subscriber is installed.
fn bench_inline_observer_paths(c: &mut Criterion) {
let paragraph = large_inline_paragraph();
let mut group = c.benchmark_group("inline_wrapping");
group.bench_function("observer_none", |b| {
b.iter(|| wrap_without_observer(black_box(&paragraph), black_box(BENCH_WIDTH)));
});
group.bench_function("tracing_observer_disabled", |b| {
b.iter(|| wrap_with_tracing_observer(black_box(&paragraph), black_box(BENCH_WIDTH)));
});
group.finish();
}

criterion_group!(benches, bench_public_document, bench_inline_observer_paths);
criterion_main!(benches);
Loading
Loading