Skip to content

Conversation

@mkroening
Copy link
Member

@mkroening mkroening commented Oct 20, 2025

This is #1991, but refactored.

Closes #1991.

@mkroening mkroening self-assigned this Oct 20, 2025
@mkroening
Copy link
Member Author

@m-mueller678, are you okay with these changes to your PR? I can't request a review from you via GitHub, but please do so anyway. :)

Copy link
Contributor

@github-actions github-actions bot left a 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: bb588b8 Previous: c98578c Performance Ratio
startup_benchmark Build Time 113.30 s 112.25 s 1.01
startup_benchmark File Size 0.89 MB 0.89 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.91 s (±0.03 s) 0.90 s (±0.03 s) 1.01
Startup Time - 4 cores 0.92 s (±0.03 s) 0.91 s (±0.03 s) 1.02
multithreaded_benchmark Build Time 114.51 s 110.14 s 1.04
multithreaded_benchmark File Size 1.00 MB 1.00 MB 1.00
Multithreaded Pi Efficiency - 2 Threads 90.03 % (±11.22 %) 92.81 % (±5.78 %) 0.97
Multithreaded Pi Efficiency - 4 Threads 45.36 % (±4.80 %) 45.41 % (±2.15 %) 1.00
Multithreaded Pi Efficiency - 8 Threads 25.57 % (±1.82 %) 25.87 % (±0.96 %) 0.99
micro_benchmarks Build Time 114.57 s 120.17 s 0.95
micro_benchmarks File Size 1.00 MB 1.00 MB 1.00
Scheduling time - 1 thread 69.02 ticks (±5.01 ticks) 74.49 ticks (±4.52 ticks) 0.93
Scheduling time - 2 threads 38.62 ticks (±6.07 ticks) 40.83 ticks (±3.64 ticks) 0.95
Micro - Time for syscall (getpid) 4.33 ticks (±0.50 ticks) 4.23 ticks (±0.55 ticks) 1.02
Memcpy speed - (built_in) block size 4096 64997.71 MByte/s (±46305.09 MByte/s) 60953.02 MByte/s (±43546.15 MByte/s) 1.07
Memcpy speed - (built_in) block size 1048576 29594.32 MByte/s (±24256.20 MByte/s) 28671.28 MByte/s (±23808.71 MByte/s) 1.03
Memcpy speed - (built_in) block size 16777216 24505.91 MByte/s (±20667.69 MByte/s) 21629.65 MByte/s (±18202.58 MByte/s) 1.13
Memset speed - (built_in) block size 4096 66008.64 MByte/s (±47022.83 MByte/s) 61354.19 MByte/s (±43777.21 MByte/s) 1.08
Memset speed - (built_in) block size 1048576 30383.11 MByte/s (±24715.88 MByte/s) 29512.24 MByte/s (±24319.97 MByte/s) 1.03
Memset speed - (built_in) block size 16777216 25305.85 MByte/s (±21208.71 MByte/s) 22292.68 MByte/s (±18645.03 MByte/s) 1.14
Memcpy speed - (rust) block size 4096 57283.81 MByte/s (±42120.98 MByte/s) 56885.86 MByte/s (±41858.13 MByte/s) 1.01
Memcpy speed - (rust) block size 1048576 29213.18 MByte/s (±24077.05 MByte/s) 28476.97 MByte/s (±23846.47 MByte/s) 1.03
Memcpy speed - (rust) block size 16777216 24246.10 MByte/s (±20211.05 MByte/s) 23424.95 MByte/s (±19786.13 MByte/s) 1.04
Memset speed - (rust) block size 4096 58104.59 MByte/s (±42652.19 MByte/s) 57746.63 MByte/s (±42387.57 MByte/s) 1.01
Memset speed - (rust) block size 1048576 30012.27 MByte/s (±24546.30 MByte/s) 29279.25 MByte/s (±24297.01 MByte/s) 1.03
Memset speed - (rust) block size 16777216 25037.95 MByte/s (±20753.77 MByte/s) 24162.64 MByte/s (±20274.04 MByte/s) 1.04
alloc_benchmarks Build Time 112.44 s 116.82 s 0.96
alloc_benchmarks File Size 0.95 MB 0.95 MB 1.00
Allocations - Allocation success 100.00 % 100.00 % 1
Allocations - Deallocation success 69.99 % (±0.24 %) 69.95 % (±0.24 %) 1.00
Allocations - Pre-fail Allocations 100.00 % 100.00 % 1
Allocations - Average Allocation time 7433.78 Ticks (±1473.13 Ticks) 10358.23 Ticks (±157.44 Ticks) 0.72
Allocations - Average Allocation time (no fail) 7433.78 Ticks (±1473.13 Ticks) 10358.23 Ticks (±157.44 Ticks) 0.72
Allocations - Average Deallocation time 933.99 Ticks (±205.61 Ticks) 894.39 Ticks (±206.54 Ticks) 1.04
mutex_benchmark Build Time 111.91 s 111.85 s 1.00
mutex_benchmark File Size 1.01 MB 1.01 MB 1.00
Mutex Stress Test Average Time per Iteration - 1 Threads 13.02 ns (±0.76 ns) 13.24 ns (±0.62 ns) 0.98
Mutex Stress Test Average Time per Iteration - 2 Threads 13.80 ns (±1.17 ns) 14.44 ns (±1.04 ns) 0.96

This comment was automatically generated by workflow using github-action-benchmark.

@mkroening mkroening force-pushed the reserve-memory branch 2 times, most recently from d6786a2 to 7d8352a Compare October 20, 2025 15:12
debug!("Memory reservation: {reservation:#x?}");
if let Ok(reserved) = PHYSICAL_FREE_LIST
.lock()
.allocate_with(|range| reservation.and(range))
Copy link
Contributor

@m-mueller678 m-mueller678 Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this does not work if reservations overlap or lie outside memory. I do not know if that can happen, but I could not find anything in the device tree docs that prohibits it.

Copy link
Member Author

@mkroening mkroening Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would overlapping reservations be an issue here? We iterate over all reservations and remove each from the list of available memory independently.

Why would reservations outside the memory be an issue? That kind of memory would not be usable anyway.

I just noticed that there might be an issue when we have one large reservation overlap with multiple memory ranges, though, since we only remove the first match. Replacing this with a while loop should help, though (d6786a2..50453a6).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think with the while loop it should work. Honestly, I find the allocate_with interface quite confusing, even more so when called in a loop. I think a comment would make sense. Something along the lines of:

While there are free intervals overlapping the reserved region, we allocate the intersection

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👍

use self::mapped_page_range_display::OffsetPageTableExt;

if !log_enabled!(Level::Debug) {
if !log_enabled!(Level::Trace) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great change, I agree this was too noisy, but was not sure if I should change it.

Copy link
Contributor

@m-mueller678 m-mueller678 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the concern about exotic memory reservations this looks good to me.

@mkroening mkroening added this pull request to the merge queue Oct 21, 2025
Merged via the queue into main with commit a9e3987 Oct 21, 2025
30 of 32 checks passed
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