pass RUSTC_HOST_FLAGS at once without the for loop

[onur-ozkan]

For obvious reasons...

[rustbot]

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[Kobzol]

Was there any specific motivation for these changes? I'm like 99% sure that this does nothing to speed up rustc invocations (I don't think there's anything to optimize in the shims, tbh), and I don't think that introducing unsafe stuff is warranted here (same with the #[inline(always)]). IMO in this case "nothing" is in fact better than "better than nothing".

[onur-ozkan]

Was there any specific motivation for these changes? I'm like 99% sure that this does nothing to speed up rustc invocations (I don't think there's anything to optimize in the shims, tbh), and I don't think that introducing unsafe stuff is warranted here (same with the #[inline(always)]). IMO in this case "nothing" is in fact better than "better than nothing".

It doesn't make sense to parse a value thousands of times when it's constant throughout the entire bootstrap invocation. For the same reason, it makes more sense to use inline(always) on the called functions. It should impact the time spent before calling the real rustc but like I said, nothing significant.

[onur-ozkan]

I don't think there's anything to optimize in the shims, tbh

I think there are still cases we could consider improving, such as parsing values from the arguments by iterating over them for each value.

[Kobzol]

What do you mean by "thousands of times"? The function is called exactly once per rustc/rustdoc invocation. Caching it in a static variable doesn't help with anything, because after rustc finishes executing, its process ends and the static variable is destroyed, along with everything else in the process. And the next rustc invocation just creates a new static variable.

The same is for the inline annotations, the compiler sees the definition of the functions, and thus doesn't need the inline annotations. It can decide on its own whether to inline or not, and in fact inlining them might not be the best choice, perf. wise. The functions are not particularly small or called often, so the function call overhead is not important But in any case, it won't actually change anything, this is completely inert in regards of rustc performance.

The time spent in the shim is inconsequential when compared to the actually invoked rustc, there's really no point in optimizing any of this.

[onur-ozkan]

What do you mean by "thousands of times"? The function is called exactly once per rustc/rustdoc invocation. Caching it in a static variable doesn't help with anything, because after rustc finishes executing, its process ends and the static variable is destroyed,

Yeah I missed that point, I was thinking that the cache lives as long as the main bootstrap process for a moment..

The same is for the inline annotations, the compiler sees the definition of the functions, and thus doesn't need the inline annotations. It can decide on its own whether to inline or not, and in fact inlining them might not be the best choice, perf. wise. The functions are not particularly small or called often, so the function call overhead is not important But in any case, it won't actually change anything, this is completely inert in regards of rustc performance.

The aim is to reduce the wrapper overhead, not to improve rustc's overall performance. If something is called frequently, wouldn’t inlining it make it cheaper than invoking it as a separate function? Doesn’t that save some extra instructions?

[Kobzol]

The aim is to reduce the wrapper overhead, not to improve rustc's overall performance. If something is called frequently, wouldn’t inlining it make it cheaper than invoking it as a separate function? Doesn’t that save some extra instructions?

Well, in theory, yes, but inlining doesn't always help performance. And maybe it was already inlined before. But what I wanted to say is that it doesn't really matter what we do in the shim. Even a check build of a hello world app performs more than 35 million instructions in rustc (according to rustc-perf). And in bootstrap we usually don't invoke just hello world :) Compiling e.g. syn is >35 billion instructions. So saving a few instructions to avoid a few function calls per rustc invocation doesn't do anything. What I'm trying to say is that this is literally a drop in the ocean and there is no point in optimizing things like this in the shim - it won't be measurable at all. It would save nanoseconds at best :)

[onur-ozkan]

I didn’t expect significant improvements here—it just made more sense to do (for instance, why iterate over a list when we can pass it directly?). If something can be done in a more sensible way, why not doing it? That's why it’s described as "better than nothing".

I get your point and I already mentioned that "no significant improvement was achieved". Unless the change is harmful to the workflow, I don't see a problem with it.

[Kobzol]

Yeah, the cmd.args change is actually a small readability improvement :) So I'm fine with that.

I still think that the #[inline(always)] annotations are completely useless here, but they also don't really hurt anything (they might raise some eye-brows, but whatever).

So feel free to r=me once CI is green.

[onur-ozkan]

I still think that the #[inline(always)] annotations are completely useless here, but they also don't really hurt anything (they might raise some eye-brows, but whatever).

I am fan of making super micro optimizations, but I don’t want to debate over something that doesn’t bring significant results. For that reason I reverted the inline change.

[onur-ozkan]

@bors r=Kobzol

[bors]

📌 Commit 4b52bbc has been approved by Kobzol

It is now in the queue for this repository.