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

Refactor lib.TagSet to use a bitmask/bitset #755

Closed
na-- opened this issue Sep 4, 2018 · 1 comment · Fixed by #1148
Closed

Refactor lib.TagSet to use a bitmask/bitset #755

na-- opened this issue Sep 4, 2018 · 1 comment · Fixed by #1148
Assignees
Milestone

Comments

@na--
Copy link
Member

na-- commented Sep 4, 2018

Probably not a huge deal in terms of performance, but the TagSet type would be much better if implemented as a bitmask/bitset (or even a plain old struct with bool fields) instead of the current map[string]bool.
We know all of the possible system tag values at compile time and we check the enabled values every time we build the tags for a metric, which would be much quicker without all of the map lookups. And making the system tag IDs in code be constants (with 1 << iota) instead of strings would probably reduce the probability of human errors when dealing with them. The only time we should use the string IDs is when we parse the user-given configuration (or inject the consolidated configuration back into the script).

@na--
Copy link
Member Author

na-- commented Jul 12, 2019

As an aside, while the bit mask can be trivially implemented by iotas, at least until we have more than 64 flags, the marshaling and unmarshaling of string values should probably happen with some sort of a go-generated code, like how we use enumer for the response types and the compression types.

@na-- na-- added this to the v0.26.0 milestone Aug 27, 2019
@cuonglm cuonglm self-assigned this Sep 3, 2019
cuonglm added a commit that referenced this issue Sep 9, 2019
Add tag set as a bitmask, as part of refactoring lib.TagSet.

Update #755
cuonglm added a commit that referenced this issue Sep 27, 2019
Add tag set as a bitmask, as part of refactoring lib.TagSet.

Update #755
cuonglm added a commit that referenced this issue Sep 30, 2019
Add tag set as a bitmask, as part of refactoring lib.TagSet.

Update #755

silent linter

fix @imiric's review comments
cuonglm added a commit that referenced this issue Oct 17, 2019
Add tag set as a bitmask, as part of refactoring lib.TagSet.

Update #755

silent linter

fix @imiric's review comments
cuonglm added a commit that referenced this issue Oct 17, 2019
Add tag set as a bitmask, as part of refactoring lib.TagSet.

Update #755

silent linter

fix @imiric's review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants