vm: Micro-optimize page rename
ClosedPublic
Actions

Authored by alc on Nov 28 2024, 5:06 PM.

Details

Reviewers

dougm
markj

Commits

rGc296ac7e0f1c: vm: Optimize page rename

Summary

Rename vm_page_rename() to vm_page_iter_rename() to reflect its reimplementation using iterators, and pass the page to this function rather than spending clock cycles looking it up. Change its return value from 0/1 to a bool.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

alc requested review of this revision.Nov 28 2024, 5:06 PM

alc created this revision.

Comments in vm_reserv.c need to be updated to reflect this change.

If you have measured the improvement, how big is it?

markj accepted this revision.Nov 28 2024, 5:14 PM

markj added inline comments.

sys/vm/vm_page.c
2072	Perhaps further clarify that failure happens only when memory allocation fails. In particular, if a page already exists at the destination, we panic. I'd also clarify that the page is passed directly only as an optimization.
2096	I believe we can strengthen this to `KASSERT((m->ref_count & VPRC_OBJREF) != 0`, as in vm_page_insert_radixdone() for example.

This revision is now accepted and ready to land.Nov 28 2024, 5:14 PM

dougm added inline comments.Nov 28 2024, 6:39 PM

sys/vm/vm_object.c
1793–1794	Fix comment.

dougm mentioned this in D47695: vm_object: iterate to add pages in reconstruction.Nov 28 2024, 6:44 PM

alc marked 3 inline comments as done.Nov 29 2024, 5:54 PM

alc added inline comments.

sys/vm/vm_page.c
2072	I didn't describe passing the page as being an optimization as I'm afraid that doing so could be misinterpreted as meaning that NULL can be passed.

Update in response to reviewer comments.

This revision now requires review to proceed.Nov 29 2024, 5:58 PM

dougm added inline comments.Nov 29 2024, 6:16 PM

sys/vm/vm_page.c
2075	I don't see a panic anywhere.
sys/vm/vm_reserv.c
681	'm' and 'new_object' are now the second and third arguments to vm_page_iter_rename, so you should be either less precise ("vm_page_iter_rename(...)") or more accurate ("vm_page_iter_rename(&old_pages, m, new_object. ...)").

In D47829#1090358, @dougm wrote:

Comments in vm_reserv.c need to be updated to reflect this change.

If you have measured the improvement, how big is it?

The conversion to iterators made vm_object_collapse_scan() and vm_object_split() slower by 14% and 5-6%, respectively. This change eliminates about half of that slowdown in each case.

Update in response to reviewer comments.

alc marked an inline comment as done.Nov 29 2024, 6:53 PM

alc added inline comments.