-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
server: fix misleading TestJemalloc result for ppc64le #46255
Conversation
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 for the fix. The CI failure looks unrelated. I'll poke CI to run again, and get this merged on green.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @petermattis)
Thanks for the review @petermattis ! |
@amitsadaphule Do you mind rebasing onto current master? The CI failures seem to be do to your old branch. |
@petermattis sure. I'll rebase the branch. |
The test failed on ppc64le since the 16KiB constant in allocateMemory didn't exceed the thread cache on ppc64le. Brief failure log shown below: ``` === RUN TestJemalloc --- FAIL: TestJemalloc (0.00s) jemalloc_test.go:37: allocated stat not incremented on allocation: 2322976 ``` Configuring jemalloc with `--disable-tcache to disable the thread cache` or flushing thread cache as `mallctl("thread.tcache.flush", NULL, NULL, NULL, 0);` showed expected behavior. But had performance and code clutter trade-offs resp. So, updated the constant to suit ppc64le which also works for x86 and others. Release justification: Fixes misleading test result for jemalloc stat on PPC Release note: None
97e3afb
to
baf61cb
Compare
@amitsadaphule Thanks. bors r+ |
Build failed |
Known / unrelated CI failure. Trying again. bors r+ |
Build failed (retrying...) |
Another unrelated CI failure (#43251). This PR doesn't want to go in. bors r+ |
Already running a review |
Build succeeded |
Fixes #43522
The test failed on ppc64le since the 16KiB constant in allocateMemory
didn't exceed the thread cache on ppc64le. Brief failure log shown below:
Configuring jemalloc with
--disable-tcache to disable the thread cache
orflushing thread cache as
mallctl("thread.tcache.flush", NULL, NULL, NULL, 0);
showed expected behavior. But had performance and code clutter trade-offs resp.
So, updated the constant to suit ppc64le which also works for x86 and others.
Release justification: Fixes misleading test result for jemalloc stat on PPC
Release note: None