-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Downgrade bundled jemalloc version #30985
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
@bors r+ Might as well run the bugs that we didn't run into. |
📌 Commit ab758e5 has been approved by |
@bors p=1 |
⌛ Testing commit ab758e5 with merge 6834008... |
💔 Test failed - auto-win-gnu-64-nopt-t |
Hm, this is... interesting. We seem to be in a bit of a quandry with respect to jemalloc on Windows. I suspect that this is jemalloc/jemalloc#310 coming to haunt us again. In the past the All in all, it appears that Thoughts @rust-lang/tools? Opinions on disabling jemalloc for Windows temporarily until we can re-upgrade again? |
Well, if it breaks stuff, we should disable it. Are we gonna lose on some important benchmark because of this? |
I suspect we'll looks on some benchmark as jemalloc is probably faster than the system allocator for at least some workloads, but I also agree that it's likely more important to work at all! |
Seems to me we should disable jemalloc on windows, yes. It'd be great if we can make a corresponding Rust issue with the details and citations you've dumped on this thread, so that we have a central place to track this. (Would this mean that we would enable the newer jemalloc elsewhere? I'm guess not, because jemalloc/jemalloc#315 doesn't appear to be specific to Windows.) |
ab758e5
to
840a1d7
Compare
Ok, I've filed an issue and disabled jemalloc on windows-gnu. r? @brson |
@bors r+ p=1 |
📌 Commit 840a1d7 has been approved by |
⌛ Testing commit 840a1d7 with merge 05756dd... |
💔 Test failed - auto-win-gnu-32-nopt-t |
840a1d7
to
66cdd85
Compare
@bors: r=brson 66cdd85 |
⌛ Testing commit 66cdd85 with merge 5a50f7f... |
💔 Test failed - auto-win-gnu-64-opt |
We've been seeing a lot of timeouts in tests on the bots and investigation ended pointing to jemalloc/jemalloc#315 as the culprit. Unfortunately it looks like that doesn't seem to have a fix on the way soon, so let's temporarily downgrade back to the previous version of jemalloc we were using (where rust-lang#30434 was the most recent upgrade)
66cdd85
to
884de56
Compare
We've been seeing a lot of timeouts in tests on the bots and investigation ended pointing to jemalloc/jemalloc#315 as the culprit. Unfortunately it looks like that doesn't seem to have a fix on the way soon, so let's temporarily downgrade back to the previous version of jemalloc we were using (where #30434 was the most recent upgrade)
We've been seeing a lot of timeouts in tests on the bots and investigation ended
pointing to jemalloc/jemalloc#315 as the culprit. Unfortunately it looks like
that doesn't seem to have a fix on the way soon, so let's temporarily downgrade
back to the previous version of jemalloc we were using (where #30434 was the
most recent upgrade)