Skip to content

bench: drop flaky isinstance, event handler, and compiler import benchmarks - #7188

Merged
masenf merged 3 commits into
reflex-dev:mainfrom
FarhanAliRaza:farhan/remove-flaky-benchmarks
Sep 18, 2026
Merged

masenf merged 3 commits into
reflex-dev:mainfrom
FarhanAliRaza:farhan/remove-flaky-benchmarks

Conversation

@FarhanAliRaza

@FarhanAliRaza FarhanAliRaza commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

What

Removes CodSpeed benchmarks that flag noise as regressions on unrelated PRs.

Removed

  • test_isinstance_container[list_dict] (tests/benchmarks/test_isinstance.py): validated a 10k-element list[dict[str, int]] through _isinstance(nested=1). It exercised the same per-element recursion as the remaining list_int, list_optional and list_typeddict params, so no coverage is lost.
  • test_from_event_type[event_handler] (tests/benchmarks/test_event_creation.py): called Event.from_event_type with a bare EventHandler. That path calls the handler to build an EventSpec, which is already covered by the event_spec and lambda_event_handler params.
  • test_from_event_type[lambda_event] (same file): called Event.from_event_type with a lambda returning an existing Event. It drifted 3-4% on unrelated PRs (ENG-12486 feat(sdk): log in, review code, and configure cloud providers #7175 and this one). The event param covers the Event pass-through branch and lambda_event_spec / lambda_event_handler cover the lambda unwrapping branch.
  • test_from_event_type[lambda_event_spec] (same file): called Event.from_event_type with a lambda returning an EventSpec. It drifted 3-4% on ENG-12486 feat(sdk): log in, review code, and configure cloud providers #7175 and on this PR. It is the event_spec path plus one lambda call, which lambda_event_handler already covers.
  • test_get_all_imports and test_collect_imports (tests/benchmarks/test_compilation.py): measured import collection alone, once via the legacy Component._get_all_imports recursion and once via a plugin walk restricted to imports. Both _complicated_page variants drift by several percent between runs. test_compile_page and test_compile_all_artifacts still walk the tree and collect imports as part of the full compile, and test_import_reflex keeps measuring cold import reflex time.

The import-only collector plugin in tests/benchmarks/fixtures.py had no other user and is removed with them.

Testing

uv run pytest tests/benchmarks
46 passed

No package source is touched, so this carries skip-changelog.

@FarhanAliRaza
FarhanAliRaza requested a review from a team as a code owner September 17, 2026 17:52
@FarhanAliRaza FarhanAliRaza added the skip-changelog For doc/internal changes label Sep 17, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

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.

No issues found across 4 files

Re-trigger cubic

@greptile-apps

greptile-apps Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The benchmark-only changes appear safe to merge, with one non-blocking stale-docstring issue remaining in the existing review thread.

Findings

  1. P2 Stale Coverage Description

Summary

This PR removes noisy CodSpeed benchmark cases and the import-only fixture that became unused.

  • Removes redundant _isinstance coverage for a large list[dict].
  • Removes flaky event-normalization benchmark parameters.
  • Removes standalone import-collection benchmarks while retaining full compilation benchmarks.
  • Removes the now-unused import collector plugin and associated imports.

Reviews (4) · Last reviewed commit: "bench: drop the flaky lambda_event_spec ..."

…hmarks

Remove test_isinstance_container[list_dict], test_from_event_type[event_handler],
test_get_all_imports and test_collect_imports, whose _complicated_page variants
flag noise as regressions. The remaining compile benchmarks still walk imports,
and test_import_reflex keeps measuring cold import time.
@FarhanAliRaza
FarhanAliRaza force-pushed the farhan/remove-flaky-benchmarks branch from 7a871ae to 03c5ca2 Compare September 17, 2026 17:57
@FarhanAliRaza FarhanAliRaza changed the title bench: drop flaky benchmarks and measure import reflex in a fresh interpreter bench: drop flaky isinstance, event handler, and compiler import benchmarks Sep 17, 2026
@codspeed

codspeed Bot commented Sep 17, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 45 untouched benchmarks
⏩ 17 skipped benchmarks1


Comparing FarhanAliRaza:farhan/remove-flaky-benchmarks (02ab2c5) with main (00228b8)

Open in CodSpeed

Footnotes

  1. 17 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Unrelated PRs (reflex-dev#7175, reflex-dev#7188) moved it by 3-4% in simulation mode. The
event param covers the Event pass-through branch and the remaining lambda
params cover the lambda unwrapping branch.
Comment thread tests/benchmarks/test_event_creation.py Outdated
It drifted 3-4% on reflex-dev#7175 and reflex-dev#7188 with no event code changed. It is the
event_spec path plus one lambda call, which lambda_event_handler already
covers.
@masenf
masenf merged commit 98f9333 into reflex-dev:main Sep 18, 2026
125 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog For doc/internal changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants