Skip to content
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

Line buffer compiler messages when possible #32486

Closed
alexcrichton opened this issue Mar 25, 2016 · 15 comments
Closed

Line buffer compiler messages when possible #32486

alexcrichton opened this issue Mar 25, 2016 · 15 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@alexcrichton
Copy link
Member

Cargo has long since been plagued with an issue like rust-lang/cargo#1198 where it executes the compiler in parallel but this can cause colors to get corrupted on the command line. The fundamental bug here is that Cargo is running rustc concurrently, but that's more of a feature than a bug, so let's try to explore other options!

One possible route to take was that we should line buffer as much as possible. With line buffering we can ensure that output may be interleaved, but it at least won't have corrupted colors. We could even go as far as to buffering entire diagnostics (which may span multiple lines).

The snag here is that this doesn't work on native Windows terminals. The way colors work there is that it's a property of the terminal itself, rather than encoded in the output stream. On Unix (and in mintty I believe) the colors are encoded in the output stream, and can hence be buffered.

So it sounds like a great way to approach this might be:

  • First, let's buffer all diagnostic messages if colors aren't enabled (where stdout is flushed after each message).
  • If we're on a native Windows terminal, we just do what we do today. Colors are better than no colors, even if sometimes they're broken due to concurrency.
  • Otherwise, colorization works by encoding into the output stream, so we can buffer entire messages (like the first bullet) with the colors, and then emit it all at once.

This should immediately improve the situation on Unix (and even add colors to MinGW). This unfortunately won't help the concurrent native Windows console experience, but there's not really much we can do there, but at least it remains status quo!

cc @Stebalien - current maintainer of term
cc @brson - from our discussion on IRC

@nikomatsakis
Copy link
Contributor

If we had coarse-grained buffering -- ideally, I think, an entire message along with its subsidiary notes and such -- that would be best, right? This interestingly interacted with the new-style error messages as well, in that our current output requires a multi-line regex to match, but emacs at least had trouble because we would flush before all parts were there, and it wouldn't know to reconsider some of the previous output when checking for a match. (The engine always matches against complete lines, but isn't really targeting multi-line regexs.) I imagine many editors are designed similarly.

@jethrogb
Copy link
Contributor

jethrogb commented May 8, 2016

Just a clarification about what has already been said:

The snag here is that this doesn't work on native Windows terminals. The way colors work there is that it's a property of the terminal itself, rather than encoded in the output stream. On Unix (and in mintty I believe) the colors are encoded in the output stream, and can hence be buffered.

Note that on Windows, even full per-process buffering and stdout-locking of “rich text" (a string abstraction which includes formatting information which on Windows would be configured at the right time) is not sufficient; there might be multiple processes outputting colored text at the same time, and the color configuration and console output functions are not synchronized. A Windows-compatible solution would require several compiler processes to coordinate a shared lock per console.

@nikomatsakis
Copy link
Contributor

Yes, @alexcrichton and I were discussed this. It seems like it would
be "ok", if overkill, to have some "rustc error lock" that all
concurrent rustc executions can grab in order to synchornize
with one another. I'm not sure of the easiest way to achieve such
a lock -- file locking? some other API?

On Sat, May 07, 2016 at 07:50:25PM -0700, jethrogb wrote:

The snag here is that this doesn't work on native Windows terminals. The way colors work there is that it's a property of the terminal itself, rather than encoded in the output stream. On Unix (and in mintty I believe) the colors are encoded in the output stream, and can hence be buffered.

Note that on Windows, even full per-process buffering and stdout-locking of “rich text" (a string abstraction which includes formatting information which on Windows would be configured at the right time) is not sufficient; there might be multiple processes outputting colored text at the same time, and the color configuration and console output functions are not synchronized. A Windows-compatible solution would require several compiler processes to coordinate a shared lock per console.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#32486 (comment)

@retep998
Copy link
Member

retep998 commented May 9, 2016

It is trivial to have an interprocess lock using CreateMutex. However the difficult part will be making the lock specific to the console screen buffer so that rustcs in different consoles don't block each other, and even two rustcs sharing the same console window but still different buffers. Also, if all the output of rustc is buffered, there would need to be a way to ensure that all the output does get written out even if rustc crashes and burns.

@alexcrichton
Copy link
Member Author

@nikomatsakis for locking on Windows we could use something like what was implemented in #33155, and we can likely eschew locking on Unix if we line-buffer everything.

@jethrogb
Copy link
Contributor

jethrogb commented May 9, 2016

Does LockFileEx work like you'd expect on console handles?

@retep998
Copy link
Member

retep998 commented May 9, 2016

One possibility is to use GetConsoleWindow to get the console window and then GetWindowThreadProcessId to get a thread and process ID that started the window. Then a name could be chosen for the named mutex based on those IDs. It isn't per console buffer, only per console window, but still better than nothing.

@jethrogb
Copy link
Contributor

jethrogb commented May 9, 2016

What's the difference between a console buffer and a console window?

@retep998
Copy link
Member

retep998 commented May 9, 2016

A console window is the physical window itself. A console buffer is the 2D grid buffer of characters. While there can be only one active console buffer, which is the one currently being displayed in the window, you can have as many console buffers as you like, and write to them even when they're not active. Stuff like the color palette, buffer size, view size, and the font are all properties of the screen buffer, not the window.

@jethrogb
Copy link
Contributor

jethrogb commented May 9, 2016

I see, so something like a screen/tmux implementation on Windows might use that, and then different rustc processes in that window would share a lock unnecessarily. I suppose that's fine as it's an unlikely situation and the locks won't be held for very long anyway.

@nikomatsakis
Copy link
Contributor

@retep998

However the difficult part will be making the lock specific to the console screen buffer so that rustcs in different consoles don't block each other

I would not bother to make the lock specific to the console screen buffer. The lock only needs to be held for the duration of printing the error to the screen so I think it's ok if we do a little more synchronization that necessary.


@alexcrichton

we can likely eschew locking on Unix if we line-buffer everything.

To be clear, I don't think line-buffering is sufficient -- that would still intermingle the lines of different error messages, right? I imagine we would want to buffer up the entire message and write it out in one chunk. That's probably good enough in practice, though I imagine if the message gets really long one could still see problems since the reader may not consume all the pending bytes in one shot.

@alexcrichton
Copy link
Member Author

Ah yeah that's write. I'm not sure if more-than-one-line buffering works in practice, but I'm also sure we could figure out some strategy for locks on Windows.

@alexcrichton
Copy link
Member Author

Er, on Unix*

@Stebalien
Copy link
Contributor

@alexcrichton there's no reason more than one line buffering shouldn't work.

@steveklabnik steveklabnik added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 9, 2017
@alexcrichton
Copy link
Member Author

I believe this has since been implemented, so closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants