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
cherry-pick from upstream jemalloc to fix profiling bug #9
Conversation
7ea3700
to
6df9179
Compare
@BusyJay , @sticnarf , this is ready for a look now that tikv/jemalloc#2 has landed. |
Travis CI is out of quota, I may need some time to migrate to github actions. |
jemalloc-sys/configure/configure
Outdated
@@ -862,6 +863,7 @@ datadir='${datarootdir}' | |||
sysconfdir='${prefix}/etc' | |||
sharedstatedir='${prefix}/com' | |||
localstatedir='${prefix}/var' | |||
runstatedir='${localstatedir}/run' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the configure file (cd jemalloc-sys/jemalloc && autoconf && cp configure ../configure/configure
) as a matter of habit when modifying autoconf-based sub modules. We can remove it if you prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's related to the version of autoconf. Unless it can fail compilation, better leave them untouch.
New CI can be triggered once it's merged master. |
6df9179
to
62fb2fe
Compare
Signed-off-by: Brennan Vincent <brennan@materialize.com>
62fb2fe
to
bc12ea1
Compare
I have reverted the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Unfortunately, this broke tikv/jemallocator, since tikv/jemalloc#2 landed with a different commit number. Sending a new PR now. |
Fixed in #12 |
This depends on tikv/jemalloc#2 , which is a cherry-pick of jemalloc/jemalloc#1604 .
See jemalloc/jemalloc#1605 for details of the bug that this fixes; basically, it makes profiling/sampling fail to work under some conditions. We hit this in practice at Materialize when attempting to switch to tikv/jemallocator.