Skip to content

Conversation

@davidmorgan
Copy link
Contributor

@davidmorgan davidmorgan commented Nov 13, 2025

Older build_runner versions stack overflow on very large max import "depth", so the other benchmark "shapes" don't work.

It would anyway be more realistic to have much more mixed imports.

So, add a "shape" with random imports. Use low-number-biased random numbers instead of flat random numbers as random flat imports are not realistic, either.

@github-actions
Copy link

PR Health

Changelog Entry ✔️
Package Changed Files

Changes to files need to be accounted for in their respective changelogs.

This check can be disabled by tagging the PR with skip-changelog-check.

@davidmorgan davidmorgan requested a review from jensjoha November 13, 2025 16:07
@davidmorgan davidmorgan marked this pull request as ready for review November 13, 2025 16:13
if (_random.nextInt(libraryNumber + 1) == 0) 'app.dart',
for (var i = 0; i != numberOfImports; ++i)
Benchmarks.libraryName(
_random.nextInt(_random.nextInt(benchmarkSize + 1) + 1) + 1,

Choose a reason for hiding this comment

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

At least give a comment about all the +1s...
As I read it, if benchmarkSize is 9 this will give a number between 1 and 10 which Benchmarks.libraryName will then stringify to "between" lib2.dart and lib11.dart which might be right, but at least doesn't seem right (to me).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, yes, it was too many +1s.

Only need one +1 to avoid passing 0 to nextInt; and libraryName does +1 as you pointed out.

Thanks.

@davidmorgan davidmorgan merged commit b9ae225 into dart-lang:master Nov 14, 2025
36 checks passed
@davidmorgan davidmorgan deleted the benchmark-tree branch November 14, 2025 09:04
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