Improve PageTableIndex and PageOffset.

[m-ou-se]

• There are now const functions to make them: new_truncate.
• The new truncating are used in VirtAddr to avoid needless checks.
• The guarantee that they are in 0..512 and 0..4096 is documented.
• This guarantee is used in Index for PageTable to avoid needless checks.
• They are now both Into, to make it easier to use them as index.

[phil-opp]

Thanks a lot for the pull request! Most of the changes look good to me, I'm just not sure about the new uses of unsafe.

While I agree that the two get_unchecked calls should be safe in this case, they're always a maintainance burden because they require a more closer review of any future changes to the PageTableIndex type. So I'm not sure whether they are worth it.

I would prefer if we could run some benchmarks before adding any unsafe blocks only for performance optimization. Maybe the compiler is able to eliminate the bounds check for the safe variant too? If not, maybe the the performance impact is negligible?

I think it's best to merge this pull request without the unsafe changes first and then discuss these changes in a followup PR.

[phil-opp]

@m-ou-se Friendly ping :).

[m-ou-se]

Pong!

[m-ou-se]

While I agree that the two get_unchecked calls should be safe in this case, they're always a maintainance burden because they require a more closer review of any future changes to the PageTableIndex type.

As the documentation now makes the guarantee that a PageTableIndex is always <512, users of this crate might also start to use unsafe indexing using it. So I don't think there would be much space for changing PageTableIndex anyway. But I definitely agree that less unsafe code is better. In this case, I'd guess that in pretty much all cases where PageTableIndex is used, the compiler can track the index back to where it is created, and should know it is <512, thus dropping any redundant asserts.

I added #[inline] to make sure the compiler can 'see' into this function from outside this create and do this optimization. Maybe all trivial/small functions in this crate should be #[inline], but that's another issue.

[phil-opp]

Thanks a lot for the update!

Maybe all trivial/small functions in this crate should be #[inline], but that's another issue.

Sounds reasonable. I think with link time optimization, the compiler does cross-crate inlining also without the attribute, but not everyone is using lto.

[phil-opp]

Released as version 0.9.1