[claude] add benchmarks website v3 design overview and plan#7756
Open
connortsui20 wants to merge 9 commits intoct/benchmarks-v3from
Open
[claude] add benchmarks website v3 design overview and plan#7756connortsui20 wants to merge 9 commits intoct/benchmarks-v3from
connortsui20 wants to merge 9 commits intoct/benchmarks-v3from
Conversation
Split the 1702-line server/src/api.rs into a directory module with one topical file per concern: dto (wire-shape structs), window (CommitWindow + ChartQuery), groups (group discovery passes), summary (v2-compatible rollups), charts (per-fact-table chart payloads + SeriesAccumulator), and filter (chip universe). The api/mod.rs surface re-exports the existing public names so callers in html.rs and tests don't have to update imports. Behaviour preserved: every wire shape, SQL query, and snapshot test matches byte-for-byte. The only structural change is a small helper push_window_limit in charts.rs that pulls the repeated CommitWindow::limit_param push out of the five chart collectors. Signed-off-by: Claude <noreply@anthropic.com>
Split the 1080-line server/src/html.rs into a directory module: - html/mod.rs — router(), UiQuery, FilterState, parse_csv, the three async page handlers, and collect_landing_groups (entry surface). - html/render.rs — render_page, theme_bootstrap_script, site_header, the SVG icon helpers, error_page, versioned_asset and STATIC_ASSET_VERSION (page chrome). - html/landing.rs — landing_body, chart_card, downsample_badge_slot, LandingGroup. - html/chart.rs — chart_body, group_body (permalink page bodies). - html/summary.rs — summary_markup + format_time_ns. - html/filter.rs — filter_dropdown, filter_row, filter_state_script. - html/toolbar.rs — per_chart_toolbar, range_strip. - html/static_assets.rs — serve_*, static_response, the include_bytes! consts, STATIC_ASSET_VERSION, versioned_asset. Behaviour preserved: every snapshot test, integration assertion, and inline JSON byte stays identical. Signed-off-by: Claude <noreply@anthropic.com>
Pull the four `*Accum` structs and their `build_*_batch` functions into a sibling `migrate/accum.rs`, leaving `migrate/mod.rs` as the orchestrator (`MigrationSummary`, `open_target_db`, `run`, `flush_all`, `apply_v2_record`, `migrate_data_jsonl`, `migrate_file_sizes`, `flush`). The four accumulators are kept as siblings rather than abstracted behind a trait. They share the dedup-on-`measurement_id` shape but their parallel-vec layouts diverge (CompressionSize uses a HashMap and has two collision semantics — replace and sum), and the existing push/build code is more legible when each accumulator is a separate, self-contained type. Behaviour preserved: every batch schema, dedup semantic, and Display output stays identical. Signed-off-by: Claude <noreply@anthropic.com>
Pull the 23 integration tests in server/tests/web_ui.rs into five
topical test crates plus a shared `common/mod.rs` fixture module:
- common/mod.rs — Server harness, seed/seed_long_history,
envelope_for/ra_envelope_for, extract_chart_data, pick_chart_slug,
pick_group_slug, group_by_name, assert_close, filter_bar_section,
filter_section, extract_chip, insta_settings, TOKEN.
- landing.rs — landing page + global filter chip tests.
- chart_api.rs — `/chart/{slug}` and `/api/chart/{slug}` tests.
- group_api.rs — `/group/{slug}`, `/api/group/{slug}`, group
summary contract tests.
- permalinks.rs — full-history inline payload, embedded filter
state, unknown-slug 404 tests.
- static_assets.rs — bundled static asset routes + compression
layer tests.
Snapshot files don't move because every `assert_snapshot!` call uses
`set_prepend_module_to_snapshot(false)` plus an explicit name, so the
snapshot key is path-independent. The four existing snapshot files
in `server/tests/snapshots/` continue to match byte-for-byte
without `INSTA_UPDATE`.
Signed-off-by: Claude <noreply@anthropic.com>
Path A from the brief (smaller diff, comments only): keep the single file and the outer IIFE, but document the implicit contracts that were previously rediscovered every time someone touched the file. Add to the top of `chart-init.js`: - A file map of the 18 sections, in source order. - A canvas state contract listing every `canvas.__bench_*` field (`__bench_chart`, `__bench_payload`, `__bench_state`, `__bench_overrides`, `__bench_strip_render`, `__bench_rebuild`, `__bench_wheel_attached`, `__bench_inline_trimmed`, `__bench_full_loaded`, `__bench_full_fetch_pending`) with a one-line description of what each holds and who writes it. - A per-card DOM contract listing every `data-role` selector (and the one `id`) the JS queries, so the implicit DOM contract with `html.rs` is visible in one place. - A factory contract on `externalTooltipHandler` describing what `canvas`, `host`, and `host.parentNode` are expected to be and what the returned handler does. The old per-function state contract block above `constructChart` becomes a one-line pointer to the consolidated top-of-file contract. Path B (split into separate static files served from /static/) was considered and rejected: introducing new `<script>` tags would change the rendered HTML and force snapshot updates, which the brief asks to avoid for this pass. No behavioural change. Asset version bump intentionally skipped — docs-only change, deployed users see byte-equivalent functionality. Signed-off-by: Claude <noreply@anthropic.com>
Update planning/AGENTS.md so the Code map table reflects the api/ and html/ directory modules, the migrate/ split, and the multi-file tests/ layout. Update planning/README.md to point the deferred N+1 follow-up at the new path (`api/charts.rs::collect_group_charts`) instead of the old `api.rs:1202-1233` line range. Signed-off-by: Claude <noreply@anthropic.com>
`cargo +nightly fmt` re-orders the use statements in `api/mod.rs` so the public re-exports are interleaved with the crate-private ones in alphabetical order across visibility modifiers, which is what nightly rustfmt's StdExternalCrate group_imports configuration produces. Signed-off-by: Claude <noreply@anthropic.com>
connortsui20
changed the title
connortsui20
changed the title
Merging this PR will degrade performance by 61.46%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | mix[0%_in/100%_out] |
228.4 µs | 277.5 µs | -17.71% |
| ⚡ | WallTime | runend[10M_i32_runlen_10] |
194.2 µs | 161.1 µs | +20.56% |
| ❌ | WallTime | mix[50%_in/50%_out] |
338.9 µs | 391.2 µs | -13.35% |
| ❌ | WallTime | dict[10M_u32_values_u16_codes] |
146.5 µs | 181.7 µs | -19.36% |
| ⚡ | WallTime | 10M_50%[5000000] |
182.8 µs | 153 µs | +19.51% |
| ⚡ | WallTime | 10M_10%[1000000] |
160 µs | 132.8 µs | +20.51% |
| ⚡ | Simulation | chunked_opt_bool_into_canonical[(10, 1000)] |
1.4 ms | 1 ms | +35.77% |
| ⚡ | Simulation | chunked_opt_bool_into_canonical[(1000, 10)] |
68.4 µs | 62 µs | +10.34% |
| ⚡ | Simulation | chunked_varbinview_into_canonical[(10, 1000)] |
2.1 ms | 1.7 ms | +20.56% |
| ⚡ | Simulation | chunked_opt_bool_into_canonical[(100, 100)] |
259.6 µs | 223.1 µs | +16.38% |
| ⚡ | Simulation | chunked_varbinview_opt_into_canonical[(10, 1000)] |
2.9 ms | 2.5 ms | +16.18% |
| ⚡ | Simulation | decompress[datetime_for_bp] |
2.4 ms | 1.6 ms | +45.58% |
| ❌ | Simulation | alp_rd_decompress_f64 |
1.1 ms | 2.4 ms | -54.94% |
| ❌ | Simulation | decompress[alp_for_bp_f64] |
1.8 ms | 2.8 ms | -36.92% |
| ❌ | Simulation | decompress_rd[f64, (10000, 0.0)] |
122.4 µs | 257.7 µs | -52.49% |
| ❌ | Simulation | decompress_rd[f32, (100000, 0.01)] |
582.8 µs | 1,374.8 µs | -57.61% |
| ❌ | Simulation | decompress_rd[f32, (10000, 0.1)] |
82.1 µs | 165.5 µs | -50.38% |
| ❌ | Simulation | decompress_rd[f64, (100000, 0.01)] |
1 ms | 2.3 ms | -56.31% |
| ❌ | Simulation | decompress_rd[f32, (100000, 0.1)] |
582.8 µs | 1,374.3 µs | -57.59% |
| ❌ | Simulation | decompress_rd[f64, (10000, 0.1)] |
122.4 µs | 258.2 µs | -52.59% |
| ... | ... | ... | ... | ... | ... |
ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.
Comparing claude/benchmarks-v3-refactor (f965259) with ct/benchmarks-v3 (6b76221)
`RUSTDOCFLAGS="-D warnings" cargo doc` fails the workspace docs build
with `unresolved link to Source::Local` on the module-level `//!`
comment in `migrate/src/source.rs`. Rustdoc can't resolve the
shorthand from the inner doc comment scope; qualify the link as
`crate::source::Source::Local` so the docs build is clean.
Verified locally with:
RUSTDOCFLAGS="-D warnings" cargo doc --profile ci --no-deps \
-p vortex-bench-server -p vortex-bench-migrate
passes cleanly.
Signed-off-by: Claude <noreply@anthropic.com>
The Rust (docs) CI job runs `RUSTDOCFLAGS="-D warnings" cargo doc`,
which promotes both `broken_intra_doc_links` and
`private_intra_doc_links` to errors. The refactor introduced eight
new failures from public docs linking to private items and an
ambiguous module/function link:
- `api/charts.rs` and `api/summary.rs` — module-level `//!` comments
linked to private types (`SeriesAccumulator`,
`query_group_has_v2_summary`). Drop the link syntax; keep them as
plain backticked names.
- `html/mod.rs` — module-level `//!` comments linked to
`[`render`]`, `[`landing`]`, `[`chart`]`, `[`summary`]`,
`[`filter`]`, `[`toolbar`]`, `[`static_assets`]`, but those
submodules are crate-private (`mod`, not `pub mod`). Drop the link
syntax and call out the privacy in the prose.
- `api/mod.rs` — `[`groups`]` was ambiguous because `groups` is both
the submodule and the axum handler function. Disambiguate every
submodule link in the list with the `mod@` prefix.
Verified locally:
RUSTDOCFLAGS="-D warnings" cargo doc --no-deps \
-p vortex-bench-server -p vortex-bench-migrate
passes cleanly, and `cargo test --profile ci --doc -p vortex-bench-server
-p vortex-bench-migrate --all-features --no-fail-fast` runs (zero
doctests, zero failures).
Signed-off-by: Claude <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Signed-off-by: Connor Tsui connor.tsui20@gmail.com