I'm going to partially revert the PR that removed the thread and add that thread removal under a
.... are you doing that with an intent to backport that change...?
not really, we already have this stack overflow in the edition, so it's a stable -> stable regression
it's likely nontrivial to backport, too
I don't object to the revert of the thread handling, but the justification is not quite clear if we don't backport, no?
backporting to 1.31 is out of the question, but on 1.32 why not if the patch is not too big
(given that we are planning to separately try to address stack-usage via stacker as well? right?)
Do we need threads for anything but the stack size? My current reading of the code seems to suggest that we only need it for stack size. So yea, reverting seems like the wrong action. Instead we should remove all the code for running a main rustc thread
FWIW that main-thread related change also broke illumos
(I believe it did, anyway)
and I’ve been wanting to revert this for a long time already, if only to see if it would fix illumos
I see no benefits whatsoever in us trying to use the main thread in anyway at all
hold on, you two are saying "remove all code for running a main rustc thread" -- you want to do that, even to the degree of not providing a
-Z flag to get the behavior desired by ishitatsuyuki on PR #48575 ?
because I thought running on the main thread was the primary goal of that PR.
(in order to ease the debugger UX for someone who does not want to deal with multithreaded execution)
I dont mind the flag Ishi wants.
but I don’t think it achieves waht you’re describing either
okay, so the "remove all code" should be interpreted as "dont make it the default behavior" ?
I did say "revert”, though that is pretty much “remove all code”…
actually, lets move this to a targetted topic
yeah so I guess the word "revert" can mean "revert the state of the code itself" (white-box) but it can also just mean "revert the default observable behavior" (black-box)
Why would illumos fail due to the lack of a thread?
That's my initial question, do we need threads at all?
if not, once we have stacker, we can remove the leftover threading code for platforms that still need threads. The tread removal PR only removes threads on a few specific platforms and only if
RUST_MIN_STACK is not specified
ah maybe I misunderstood your point from the start here
so you are suggesting that we keep the behavior that ishitatsuyuki desires, and just rely on stacker and thus not have to worry about RUST_MIN_STACK ?
I think this comment from eddyb was one motivation for going back to spawning a separate thread: https://github.com/rust-lang/rust/pull/48575#issuecomment-380635967
but I assume you've already read all the comments there, so ... have you posted counter-arguments somewhere?
Oh oops, I forgot about that
ah there's my answer
I did read that, and then forgot about it over the last two days
So I guess we're back to reverting
right. but its still not a critical task. just something to try to do when one has time/inclination
And yeah, I prefer Ctrl+C plus bt to investigate performance issues, so it's a little more convenient if it's kept as a flag
@Tatsuyuki Ishi do you want to prep a PR for that? I lean against but if the PR is small and simple it also doesn't seem to do much harm
might be easier to have the discussion if we had a PR to examine
I don't have time for the following two weeks but I will have some after. The PR should be small against a non-reverted revision (otherwise it's adding the code back again so not trivial).
What I'm feeling is that stack probe stuffs are too hard to mess with and even the current implementation cannon be called right for every platform. Do you have any other points against keeping this under the flag? It seems the convenience isn't that much when there are more trade offs.
(for reference, here is context of niko's quote.)