Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add new command line options to enable periodic concurrency mode #391

Merged
merged 24 commits into from
Sep 27, 2023
Merged

Add new command line options to enable periodic concurrency mode #391

merged 24 commits into from
Sep 27, 2023

Conversation

nv-hwoo
Copy link
Contributor

@nv-hwoo nv-hwoo commented Sep 5, 2023 •
edited
Loading

Add two new command line options to enable periodic concurrency mode in Perf Analyzer:

  • --periodic-concurrency-range=<start:end:step>
  • --request-period=<n>
  • --request-parameter=<name:value:type>

The new command line options will users to launch Perf Analyzer in periodic concurrency mode and can be used as in the following command:

perf_analyzer -m <model_name> -i grpc --aync --streaming \
    --profile-export-file profile.json \
    --periodic-concurrency-range 20:400:10 \
    --request-period 50 \
    --request-parameter max_tokens:256:int

Note
The periodic concurrency mode only supports the decoupled models, so the user must use gRPC streaming in order to launch it on PA. Additionally, the user must also specify a file where PA could dump all the profiled data since there will be no command line outputs that display the profile statistics as in other modes.

@nv-hwoo nv-hwoo force-pushed the hwoo-add-new-options branch from a650130 to cce29f3 Compare September 5, 2023 16:46
@debermudez
Copy link
Contributor

debermudez commented Sep 5, 2023 •
edited
Loading

Given this:

The periodic concurrency mode only supports the decoupled models

can we set the necessary options and print a message to the user?

debermudez
debermudez requested changes Sep 5, 2023
View reviewed changes
Copy link
Contributor

@debermudez debermudez left a comment

Choose a reason for hiding this comment

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

Looks good so far. Couple comments is all.

matthewkotila
matthewkotila reviewed Sep 5, 2023
View reviewed changes
src/c++/perf_analyzer/command_line_parser.cc Outdated Show resolved Hide resolved
src/c++/perf_analyzer/command_line_parser.cc Outdated Show resolved Hide resolved
src/c++/perf_analyzer/command_line_parser.cc Outdated Show resolved Hide resolved
src/c++/perf_analyzer/command_line_parser.cc Outdated Show resolved Hide resolved
src/c++/perf_analyzer/command_line_parser.cc Outdated Show resolved Hide resolved
src/c++/perf_analyzer/docs/cli.md Outdated Show resolved Hide resolved
src/c++/perf_analyzer/docs/cli.md Outdated Show resolved Hide resolved
src/c++/perf_analyzer/docs/cli.md Outdated Show resolved Hide resolved
src/c++/perf_analyzer/test_command_line_parser.cc Show resolved Hide resolved
matthewkotila
matthewkotila previously approved these changes Sep 5, 2023
View reviewed changes
@nv-hwoo nv-hwoo force-pushed the hwoo-add-new-options branch from ee7db60 to 287b0c5 Compare September 21, 2023 20:24
@nv-hwoo nv-hwoo force-pushed the hwoo-add-new-options branch from ae77dc6 to 8156127 Compare September 25, 2023 23:23
@nv-hwoo
Add macros to reuse it for checking range options
03d45ea
@nv-hwoo
Add tests for periodic-concurrency-range option
842470d
@nv-hwoo
Add periodic-concurrency-range and request-period options
8e07edf
@nv-hwoo
Add doc for periodic-concurrency-range and request-period
4006100
@nv-hwoo
Add test for request-period option
6559332
@nv-hwoo
Revert macro and add reusable test function
7783853
@nv-hwoo
Add more tests
1edad2f
@nv-hwoo
Small refactor
e68f378
@nv-hwoo
Refactor a subcase
5fc5064
@nv-hwoo
Require bi-directional gRPC streaming for periodic concurrency mode
b060669
@nv-hwoo
Address feedback
be7a441
@nv-hwoo
Refine the error message
8b458a9
@nv-hwoo
Add bi-directional gRPC streaming options for periodic concurrency mode
53b653a
@nv-hwoo
Add request-parameter option and refactor
96758ec
@nv-hwoo
Refactor
fed7566
@nv-hwoo
Add valid case for request-parameter option
336c8bf
@nv-hwoo
Add --request-parameter doc and edit periodic concurrency description
99e6366
@nv-hwoo
Custom request parameter is currently only supported by gRPC
44bfa61
@matthewkotila matthewkotila dismissed their stale review September 27, 2023 00:33

Rebase/new changes

@nv-hwoo nv-hwoo force-pushed the hwoo-add-new-options branch from 8156127 to 2064297 Compare September 27, 2023 00:37
@nv-hwoo nv-hwoo merged commit 2b522e6 into periodic-concurrency-mode Sep 27, 2023
3 checks passed
@nv-hwoo nv-hwoo deleted the hwoo-add-new-options branch September 27, 2023 21:20
matthewkotila pushed a commit that referenced this pull request Oct 7, 2023
@nv-hwoo @matthewkotila
Add new command line options to enable periodic concurrency mode (#391)
276088b 
* Add macros to reuse it for checking range options

* Add tests for periodic-concurrency-range option

* Add periodic-concurrency-range and request-period options

* Add doc for periodic-concurrency-range and request-period

* Add test for request-period option

* Revert macro and add reusable test function

* Add more tests

* Small refactor

* Refactor a subcase

* Require bi-directional gRPC streaming for periodic concurrency mode

* Address feedback

* Refine the error message

* Add bi-directional gRPC streaming options for periodic concurrency mode

* Add request-parameter option and refactor

* Refactor

* Add valid case for request-parameter option

* Add --request-parameter doc and edit periodic concurrency description

* Custom request parameter is currently only supported by gRPC

* Parse and store the type of request parameter

* Add checks between act vs. exp

* Remove uint type and rebase

* Change doc

* Minor fix

* Address feedback
matthewkotila pushed a commit that referenced this pull request Oct 7, 2023
@nv-hwoo @matthewkotila
Add new command line options to enable periodic concurrency mode (#391)
fd77ba8 
* Add macros to reuse it for checking range options

* Add tests for periodic-concurrency-range option

* Add periodic-concurrency-range and request-period options

* Add doc for periodic-concurrency-range and request-period

* Add test for request-period option

* Revert macro and add reusable test function

* Add more tests

* Small refactor

* Refactor a subcase

* Require bi-directional gRPC streaming for periodic concurrency mode

* Address feedback

* Refine the error message

* Add bi-directional gRPC streaming options for periodic concurrency mode

* Add request-parameter option and refactor

* Refactor

* Add valid case for request-parameter option

* Add --request-parameter doc and edit periodic concurrency description

* Custom request parameter is currently only supported by gRPC

* Parse and store the type of request parameter

* Add checks between act vs. exp

* Remove uint type and rebase

* Change doc

* Minor fix

* Address feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewkotila matthewkotila matthewkotila approved these changes

@debermudez debermudez debermudez approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nv-hwoo @debermudez @matthewkotila