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

[ML] Always use Boost unordered maps/sets #1647

Merged
merged 1 commit into from
Jan 8, 2021

Conversation

droberts195
Copy link
Contributor

The memory instrumentation in core::CMemory exists for
boost::unordered_set and boost::unordered_map, but not
the std equivalents. It is better that we switch all
uses in the codebase at the same time (or even switch
to completely different hash container implementations).

This change switches the few cases of std::unordered_map
and std::unordered_set to the boost versions. This may
cause some increases in reported memory usage, where these
containers are in memory instrumented classes. It won't
increase actual memory usage (at least not significantly),
just make the reporting more accurate.

The memory instrumentation in core::CMemory exists for
boost::unordered_set and boost::unordered_map, but not
the std equivalents.  It is better that we switch all
uses in the codebase at the same time (or even switch
to completely different hash container implementations).

This change switches the few cases of std::unordered_map
and std::unordered_set to the boost versions.  This may
cause some increases in reported memory usage, where these
containers are in memory instrumented classes.  It won't
increase actual memory usage (at least not significantly),
just make the reporting more accurate.
Copy link
Contributor

@tveasey tveasey left a comment

Choose a reason for hiding this comment

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

LGTM

@droberts195 droberts195 merged commit d2b8d9b into elastic:master Jan 8, 2021
@droberts195 droberts195 deleted the always_use_boost_unordered branch January 8, 2021 15:14
droberts195 added a commit to droberts195/ml-cpp that referenced this pull request Jan 11, 2021
The memory instrumentation in core::CMemory exists for
boost::unordered_set and boost::unordered_map, but not
the std equivalents.  It is better that we switch all
uses in the codebase at the same time (or even switch
to completely different hash container implementations).

This change switches the few cases of std::unordered_map
and std::unordered_set to the boost versions.  This may
cause some increases in reported memory usage, where these
containers are in memory instrumented classes.  It won't
increase actual memory usage (at least not significantly),
just make the reporting more accurate.

Backport of elastic#1647
droberts195 added a commit that referenced this pull request Jan 11, 2021
The memory instrumentation in core::CMemory exists for
boost::unordered_set and boost::unordered_map, but not
the std equivalents.  It is better that we switch all
uses in the codebase at the same time (or even switch
to completely different hash container implementations).

This change switches the few cases of std::unordered_map
and std::unordered_set to the boost versions.  This may
cause some increases in reported memory usage, where these
containers are in memory instrumented classes.  It won't
increase actual memory usage (at least not significantly),
just make the reporting more accurate.

Backport of #1647
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.

2 participants