-
Notifications
You must be signed in to change notification settings - Fork 89
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
Uhyve: Speed up serial output #1468
Conversation
9437f94
to
ea7ebb9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel good about including heapless
since we have come so far without it, but I can't think of anything better. 👍
Regarding the general approach, it would be a lot nicer to have a proper io::BufWriter
, but core::io
is too far away still. (The most up-to-date approach I could find is wcampbell0x2a/no-std-io2, and even that has issues.
So leaving io::BufWriter
aside, it might still be worthwhile to have this buffering strategy take place outside arch
such that all serial ports can take advantage of it. For those that can't, the overhead will probably not be too bad, I think. Avoiding the \n
check here by extracting this into external flush
calls would also be nice sometime in the future when we have nice core::io
abstractions.
I agree with all your points. Especially, the unification of some IO functions across the architectures is desirable. But I'd still propose to merge this now and enhance later on. |
Yeah, sure, but this is blocked on #1393. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you rebase this, since #1393 is merged? :)
f793bb4
to
766972f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to move this to the arch-independent part Console
(src/console.rs#L15-L17)?
That would drastically reduce the code duplication, make the code simpler, and avoid clippy::large_enum_variant
. The additional overhead of doing an extra copy for byte-wise serial devices is probably negligible, I'd expect.
febbd13
to
383d77f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Will merge after the CI on the first commit is green. :)
5e75e62
to
b8cfb3b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The output of the second commit is wrong. I'll look for the new line issue tomorrow.
1e64ff2
to
6004cfb
Compare
SerialBufferWrite
hypercall instead of theUart
hypercall: This reduces the number of serial output related hypercalls by ~3x.Attention: