-
Notifications
You must be signed in to change notification settings - Fork 109
style: improve user-facing kernel messages #1974
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
base: main
Are you sure you want to change the base?
Conversation
fefcf37
to
a165dcb
Compare
I went a bit overboard with this 2.5 hour effort, but this fixes a massive pet peeve I've had for a while. Given my lack of familiarity with some components, some mistakes may be present. (I also deliberately didn't go as far enough as to try to make things like punctuation/capitalization consistent :D) |
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.
Benchmark Results
Benchmark | Current: 57d104e | Previous: 565ef14 | Performance Ratio |
---|---|---|---|
startup_benchmark Build Time | 107.60 s |
113.59 s |
0.95 |
startup_benchmark File Size | 0.91 MB |
0.91 MB |
1.00 |
Startup Time - 1 core | 0.90 s (±0.02 s) |
0.90 s (±0.03 s) |
1.01 |
Startup Time - 2 cores | 0.92 s (±0.02 s) |
0.90 s (±0.02 s) |
1.03 |
Startup Time - 4 cores | 0.93 s (±0.03 s) |
0.91 s (±0.02 s) |
1.02 |
multithreaded_benchmark Build Time | 111.28 s |
113.29 s |
0.98 |
multithreaded_benchmark File Size | 1.01 MB |
1.01 MB |
1.00 |
Multithreaded Pi Efficiency - 2 Threads | 2.65 % (±12.71 %) |
4.07 % (±19.55 %) |
0.65 |
Multithreaded Pi Efficiency - 4 Threads | 1.47 % (±7.08 %) |
1.94 % (±9.32 %) |
0.76 |
Multithreaded Pi Efficiency - 8 Threads | 0.88 % (±4.21 %) |
1.13 % (±5.41 %) |
0.78 |
micro_benchmarks Build Time | 104.89 s |
116.23 s |
0.90 |
micro_benchmarks File Size | 1.01 MB |
1.01 MB |
1.00 |
Scheduling time - 1 thread | 2.49 ticks (±11.94 ticks) |
2.76 ticks (±13.23 ticks) |
0.90 |
Scheduling time - 2 threads | 1.54 ticks (±7.39 ticks) |
1.64 ticks (±7.88 ticks) |
0.94 |
Micro - Time for syscall (getpid) | 0.10 ticks (±0.50 ticks) |
0.16 ticks (±0.77 ticks) |
0.65 |
Memcpy speed - (built_in) block size 4096 | 1838.24 MByte/s (±8823.53 MByte/s) |
1474.06 MByte/s (±7075.47 MByte/s) |
1.25 |
Memcpy speed - (built_in) block size 1048576 | 786.27 MByte/s (±3774.10 MByte/s) |
723.43 MByte/s (±3472.47 MByte/s) |
1.09 |
Memcpy speed - (built_in) block size 16777216 | 230.27 MByte/s (±1105.28 MByte/s) |
221.43 MByte/s (±1062.86 MByte/s) |
1.04 |
Memset speed - (built_in) block size 4096 | 1558.44 MByte/s (±7480.52 MByte/s) |
1578.95 MByte/s (±7578.95 MByte/s) |
0.99 |
Memset speed - (built_in) block size 1048576 | 998.23 MByte/s (±4791.50 MByte/s) |
974.85 MByte/s (±4679.27 MByte/s) |
1.02 |
Memset speed - (built_in) block size 16777216 | 978.24 MByte/s (±4695.55 MByte/s) |
971.72 MByte/s (±4664.27 MByte/s) |
1.01 |
Memcpy speed - (rust) block size 4096 | 1212.12 MByte/s (±5818.18 MByte/s) |
1200.00 MByte/s (±5760.00 MByte/s) |
1.01 |
Memcpy speed - (rust) block size 1048576 | 781.48 MByte/s (±3751.10 MByte/s) |
751.22 MByte/s (±3605.84 MByte/s) |
1.04 |
Memcpy speed - (rust) block size 16777216 | 230.01 MByte/s (±1104.06 MByte/s) |
212.39 MByte/s (±1019.46 MByte/s) |
1.08 |
Memset speed - (rust) block size 4096 | 1967.21 MByte/s (±9442.62 MByte/s) |
1578.95 MByte/s (±7578.95 MByte/s) |
1.25 |
Memset speed - (rust) block size 1048576 | 1002.56 MByte/s (±4812.27 MByte/s) |
994.58 MByte/s (±4773.98 MByte/s) |
1.01 |
Memset speed - (rust) block size 16777216 | 954.01 MByte/s (±4579.22 MByte/s) |
863.20 MByte/s (±4143.37 MByte/s) |
1.11 |
alloc_benchmarks Build Time | 103.65 s |
103.60 s |
1.00 |
alloc_benchmarks File Size | 0.97 MB |
0.97 MB |
1.00 |
Allocations - Allocation success | 2.00 % (±13.86 %) |
2.00 % (±13.86 %) |
1 |
Allocations - Deallocation success | 1.40 % (±9.68 %) |
1.40 % (±9.67 %) |
1.00 |
Allocations - Pre-fail Allocations | 2.00 % (±13.86 %) |
2.00 % (±13.86 %) |
1 |
Allocations - Average Allocation time | 228.02 Ticks (±1580.07 Ticks) |
282.08 Ticks (±1954.68 Ticks) |
0.81 |
Allocations - Average Allocation time (no fail) | 228.02 Ticks (±1580.07 Ticks) |
282.08 Ticks (±1954.68 Ticks) |
0.81 |
Allocations - Average Deallocation time | 13.44 Ticks (±93.11 Ticks) |
14.38 Ticks (±99.64 Ticks) |
0.93 |
mutex_benchmark Build Time | 104.26 s |
103.90 s |
1.00 |
mutex_benchmark File Size | 1.02 MB |
1.02 MB |
1.00 |
Mutex Stress Test Average Time per Iteration - 1 Threads | 0.26 ns (±1.80 ns) |
0.26 ns (±1.80 ns) |
1 |
Mutex Stress Test Average Time per Iteration - 2 Threads | 0.30 ns (±2.08 ns) |
0.30 ns (±2.08 ns) |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
521890b
to
2b10b81
Compare
- Move subjects to beginning of each message - Unify certain messages across architecture-specific implementations - State the component/task at the beginning of most messages (before verbs) to make log scrolling and pinpointing failures easier - Grammar-related improvements - Increase consistency of used wording - In some cases, display relevant variables - Replace debug! with error! before errors are returned. - Replace some debug! instances with trace! - Decrease the length of some sentences - Make messages of standard actions (e.g. driver init) consistent (mostly.) - Correct mentions of RTL8139 in the driver's messages - Add more variables inside of placeholders This change is not intended to be perfect and does not improve ALL messages, with many changes being highly subjective. It has a primary focus on debugging information and drivers. The primary motivation for this change is mostly the high amount of time spent trying to understand sequences of actions (and e.g. where specifically the booting of a kernel may break), while also assisting those that do not have a full picture of the kernel's source code. Some messages were not modified to include the name of the component, as it may be self-evident depending on the context and the debugging activity (see: src/drivers/gem.rs) and increase the mental noise, rather than decrease it.
2b10b81
to
57d104e
Compare
@mkroening Would you like me to rebase this PR (and break out the commits)? |
What do you mean by breaking out the commits? You can rebase if you like, but I still need to write down my opinion about this before this is ready to merge. :D |
This change is not intended to be perfect and does not improve ALL messages, with many changes being highly subjective.
It has a primary focus on debugging information and drivers. The primary motivation for this change is mostly the high amount of time spent trying to understand sequences of actions (and e.g. where specifically the booting of a kernel may break), while also assisting those that do not have a full picture of the kernel's source code.
Some messages were not modified to include the name of the component, as it may be self-evident depending on the context and the debugging activity (see: src/drivers/gem.rs) and increase the mental noise, rather than decrease it.