Skip to content

Improvements to throughput benchmark - #502

Merged
tlwillke merged 2 commits into
mainfrom
tput-bench-update
Jul 19, 2025
Merged

Improvements to throughput benchmark#502
tlwillke merged 2 commits into
mainfrom
tput-bench-update

Conversation

@tlwillke

Copy link
Copy Markdown
Collaborator
  • Warmup now uses random vectors instead of a split of the test vectors
  • In addition to Avg QPS, can report Median QPS and Max QPS
  • Number of warmup and test passes is configurable
  • Overloaded QueryExecutor to accept random vector queries

@MarkWolters MarkWolters 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.

Looks good, I would just like to understand why we are modifying this process to use a randomly generated vector rather than the set of query vectors from the dataset.

IntStream.range(0, totalQueries)
.parallel()
.forEach(k -> {
// Generate a random vector

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.

If the associated ConfiguredSystem has a Dataset already why are we generating a random vector rather than using the queryVector? I understand this works with the change to QueryExecutor, I'm just not clear on why we are making this change?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

There are often very few queries in a dataset. Best practice is to not use the same vectors for warmup that you use for testing (due to potential caching, fitting, etc.). Using the query set reduces the split available for testing. Now, we are using random, since it really doesn't matter what you run through for warmup.

@tlwillke
tlwillke requested a review from MarkWolters July 18, 2025 21:26

@MarkWolters MarkWolters 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.

LGTM

@tlwillke
tlwillke merged commit 26e2478 into main Jul 19, 2025
@tlwillke
tlwillke deleted the tput-bench-update branch July 19, 2025 01:33
pkolaczk pushed a commit that referenced this pull request Sep 16, 2025
* Improved throughput benchmark, adding random vector warmups, median and max QPS options.

* Added std dev measure.
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