Skip to content

Conversation

@PallHaraldsson
Copy link
Contributor

@PallHaraldsson PallHaraldsson commented Oct 25, 2024

Partial revert of #56308 and precompiles.

Would be nice to backport to 1.11.2 since printing common enough, and precompiles help it.

@PallHaraldsson PallHaraldsson changed the title Revert "Faster println()" Smaller "Faster println()" (partial revert of that brand-new addition) Oct 25, 2024
@PallHaraldsson
Copy link
Contributor Author

PallHaraldsson commented Oct 25, 2024

@KristofferC this smaller revert likely better to backport to 1.11.2 (though I'm pretty sure I saw stack-allocation go away, not dreamt it... until I figure out how, leave code for it out).

Not addressing the small regression, unrelated to this: #56326 (comment)

@PallHaraldsson PallHaraldsson mentioned this pull request Oct 25, 2024
43 tasks
@PallHaraldsson PallHaraldsson changed the title Smaller "Faster println()" (partial revert of that brand-new addition) Smaller "Faster println()" (partial revert of that brand-new addition) and precompiles Oct 26, 2024
@fingolfin
Copy link
Member

This PR has no explanation as to what motivates it: what problem does it address respectively what does it enhance? Please clarify that, e.g. by editing the PR description.

@PallHaraldsson
Copy link
Contributor Author

PallHaraldsson commented Oct 27, 2024

It's a partial revert of my merged PR that same day. I realized the extra code isn't needed so this is more like status quo of last week.

The extra code isn't a bug per se, but would require compilation, so better with precompiles or just not have it.

I also slipped in some good (IMHO) precompiles.

I think this can uncontroversially merged.

It passes CI (Windows must be false alarm like for musl):

Process failed to exit within 10800s, requesting termination and coredump (SIGQUIT) of PID 6540.

@PallHaraldsson PallHaraldsson marked this pull request as draft October 27, 2024 21:22
@PallHaraldsson PallHaraldsson marked this pull request as ready for review October 27, 2024 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants