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.

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

Do not poll counters in bulk mode during initialization for objects that support bulk per CLI option #1437

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

stephenxs
Copy link
Contributor

Do not poll counters in bulk mode during initialization for objects that support bulk per CLI option

  1. Read the counter groups that support bulk mode from CONFIG_DB.DEVICE_METADATA|localhost.supporting_bulk_counter_groups
  2. Generate CLI options to syncd
  3. Based on CLI options syncd records the counter groups that support bulk mode. For those objects, we do not need to poll them in bulk mode during initialization

Copy link
Collaborator

@kcudnik kcudnik left a comment

Choose a reason for hiding this comment

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

see comments

syncd/CommandLineOptions.cpp Outdated Show resolved Hide resolved
syncd/FlexCounter.cpp Outdated Show resolved Hide resolved
syncd/FlexCounter.cpp Show resolved Hide resolved
syncd/FlexCounter.cpp Outdated Show resolved Hide resolved
syncd/FlexCounter.cpp Outdated Show resolved Hide resolved
syncd/FlexCounter.h Outdated Show resolved Hide resolved
syncd/FlexCounter.h Outdated Show resolved Hide resolved
syncd/FlexCounter.h Outdated Show resolved Hide resolved
kcudnik
kcudnik previously approved these changes Oct 21, 2024
@stephenxs stephenxs force-pushed the not-poll-bulk-init branch 2 times, most recently from 67b848a to 8e61f47 Compare October 22, 2024 02:07
@kcudnik
Copy link
Collaborator

kcudnik commented Oct 23, 2024

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephenxs
Copy link
Contributor Author

@kcudnik I added UT to meet coverage.

@stephenxs
Copy link
Contributor Author

UT failed due to a conflict with another PR. Fixing.

Signed-off-by: Stephen Sun <stephens@nvidia.com>
…s supported

Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs
Copy link
Contributor Author

Same infra issue as that in another PR

vslib/.libs'
+ for dir in ${gcov_dirs}
++ dirname common/.libs
+ source_dir=common
+ output_file=coverage-common.json
+ gcovr --exclude-unreachable-branches --json-pretty -o coverage-common.json --object-directory common common/.libs
+ for dir in ${gcov_dirs}
++ dirname common/c-api/.libs
+ source_dir=common/c-api
+ output_file=coverage-common/c-api.json
+ gcovr --exclude-unreachable-branches --json-pretty -o coverage-common/c-api.json --object-directory common/c-api common/c-api/.libs
usage: gcovr [options] [search_paths...]
gcovr: error: argument -o/--output: Could not create output file 'coverage-common/c-api.json': No such file or directory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants