Page MenuHomeFreeBSD

Speed up iteration in vm_object_madvise().
ClosedPublic

Authored by markj on Jan 9 2017, 2:16 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Jun 9, 2:28 AM
Unknown Object (File)
Mon, Jun 3, 12:46 PM
Unknown Object (File)
Sun, Jun 2, 3:11 AM
Unknown Object (File)
May 19 2024, 9:49 AM
Unknown Object (File)
May 8 2024, 3:58 PM
Unknown Object (File)
May 8 2024, 2:56 PM
Unknown Object (File)
May 8 2024, 7:43 AM
Unknown Object (File)
Mar 19 2024, 9:27 PM
Subscribers
None

Details

Summary

Some tracing indicates that vm_object_madvise(MADV_FREE) is frequently
invoked by jemalloc on anonymous objects with no backing object. With
r311346 this occurs during each execve as well. In this case we can
iterate over the range more quickly by using the object's memq rather
than performing a radix tree lookup on each pindex in the range.
Moreover, we can skip over non-resident subranges in a single iteration.
This case occurs frequently with both execve and jemalloc - firefox, for
instance, appears to frequently purge arenas using mallctlbymib(3), and
the corresponding VA ranges are not always backed by resident pages.

While here, rename "advise" to "advice" for consistency with related
functions and because it makes more sense, and move MADV_WILLNEED
handling into vm_page_advise() (vm_object_madvise() is its only caller).

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj retitled this revision from to Speed up iteration in vm_object_madvise()..
markj edited the test plan for this revision. (Show Details)
markj updated this object.
kib edited edge metadata.
kib added inline comments.
sys/vm/vm_object.c
1148 ↗(On Diff #23752)

If you write these lines as

} else if ((m = vm_page_lookup(tobject, tpindex)) == NULL) {

you can keep existing indent (reduce indent in the patch).

This revision is now accepted and ready to land.Jan 9 2017, 1:53 PM
markj edited edge metadata.

Reduce indentation.

This revision now requires review to proceed.Jan 10 2017, 4:28 AM
kib edited edge metadata.
This revision is now accepted and ready to land.Jan 10 2017, 9:28 AM
sys/vm/vm_object.c
1145 ↗(On Diff #23813)

Isn't the assignment to "tpindex" redundant?

sys/vm/vm_object.c
1145 ↗(On Diff #23813)

Yes, you're right. It made sense in an earlier revision where I didn't set tpindex until this point - we must have tpindex = m->pindex in order for the swap_pager_freespace() call at the end of the loop to be correct.

I'll change it to "pindex = tpindex".

markj edited edge metadata.

Remove a redundant assignment.

This revision now requires review to proceed.Jan 15 2017, 1:17 AM
alc edited edge metadata.

This change looks okay. Go ahead and commit it. After that, please take a look at how vm_object_unwire() is written. The same approach might be applicable here.

This revision is now accepted and ready to land.Jan 15 2017, 2:50 AM
This revision was automatically updated to reflect the committed changes.

To be clear, my point is that I think that you can always avoid the vm_page_lookup() on the top-level object, and not just in the case that there is no backing object.

In D9098#189866, @alc wrote:

To be clear, my point is that I think that you can always avoid the vm_page_lookup() on the top-level object, and not just in the case that there is no backing object.

Ok, I see. I'll give this a try.