Skip to content

Conversation

@nvpohanh
Copy link
Contributor

@nvpohanh nvpohanh commented Jun 11, 2020

Changes:

  • Added a LoadGen benchmark to test LoadGen throughput.
  • Skip the calling of sleep_until() if in Server scenario and if the current time has already passed the current query's scheduled time.
  • Implemented server_coalesce_queries flag. When this flag is set to true, LoadGen will issue multiple samples per query when the current time has already passed multiple queries' scheduled timestamps.
    • No change to default behavior, unless you opt in this feature by setting the flag to true.
    • Latency measurements are still done on per-sample basis.

@nvpohanh nvpohanh changed the title [WORK IN PROGRESS] Implement LoadGen server_coalesce_queries flag Implement LoadGen server_coalesce_queries flag Jun 11, 2020
@nvpohanh
Copy link
Contributor Author

Unfortunately, the multi-thread IssueQuery() needs to wait until tomorrow.... Still debugging :(

@christ1ne
Copy link
Contributor

christ1ne commented Jun 16, 2020

How do I turn this feature on and off by a user?

@nvpohanh
Copy link
Contributor Author

@christ1ne By default it's off. To turn on, set the server_coalesce_queries in TestSettings to true.

@nvpohanh nvpohanh requested a review from tjablin June 18, 2020 18:10
@nvpohanh
Copy link
Contributor Author

@christ1ne @guschmue @tjablin Is it fine if we merge this first? I think this feature has less concern and it will make #620 look cleaner. Thanks!

@christ1ne
Copy link
Contributor

fine with me

@nvpohanh
Copy link
Contributor Author

@guschmue @tjablin Could we merge this if there is no objection? Thanks!

@guschmue
Copy link
Contributor

@tjablin ... good with it?

@aaronzhongii
Copy link
Contributor

Test at my local, no obvious performance degradation observed when this patch is applied and whether server_coalesce_queries flag is used or not.

@nvpohanh
Copy link
Contributor Author

@tjablin Sorry for pinging. I have addressed all your comments. Could we merge this PR? Thanks

@christ1ne christ1ne merged commit 814ba38 into mlcommons:master Jun 30, 2020
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.

5 participants