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

no longer eagerly use tcmalloc if it's found #1363

Merged
merged 1 commit into from
Jul 7, 2023
Merged

Conversation

spoonincode
Copy link
Member

Since EOSIO 1.0 tcmalloc (from gperftools) would be used if it was found during cmake configuration. Despite that, we've never once had tcmalloc enabled in any sort of CI. I know in certain historical configurations, such as Ubuntu 16.04, the tcmalloc available from the standard package repo caused a dead lock in one of the unit tests (of course, the fault certainly could be have in the test, hard to say).

The benefit of using tcmalloc is supposed improved performance, but we don't have any recent data (or maybe any data ever) to back up that claim.

It seems a little reckless to eagerly enable features like this that we don't actually test ourselves. So move tcmalloc usage behind a cmake option that is defaulted to off. Using tcmalloc is still useful for leap developers due to its profiler, etc.

A nice side effect is this may improve the reliability of the proposed automatic database migration approach too.

Copy link
Member

@heifner heifner left a comment

Choose a reason for hiding this comment

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

Do we want to backport this?

@spoonincode
Copy link
Member Author

Do we want to backport this?

I was tempted to suggest that when it seemed the recent calloc() change was causing problems in tcmalloc. But since that doesn't appear to be the case any longer, maybe not really important to backport at this point. (I still would like to take it for 5.0+ due to the reasons originally documented)

@spoonincode spoonincode merged commit 18617a5 into main Jul 7, 2023
21 checks passed
@spoonincode spoonincode deleted the no_eager_tcmalloc branch July 7, 2023 17:56
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.

3 participants