To improve performance slightly, use vm_radix_iter_lookup_ge to skip over several missing pages at once in grab_valid_iter().
Details
Details
- Reviewers
alc markj kib - Commits
- rG6dd1c0643e9c: vm_page: use lookup_ge in grab_valid_iter()
Buildworld testing shows this code rarely used after startup. So, the numbers before world building starts are
Original:
debug.counters.grab_valid_cycles: 323418
debug.counters.grab_valid_calls: 122
Modified;
debug.counters.grab_valid_cycles: 298258
debug.counters.grab_valid_calls: 121
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
@alc advises that having lookup_range return a positive value in this case will be rare, and testing shows that. So, instead of using lookup_range to find a bunch of pages, try using lookup_ge to quickly identify a range of missing pages.
Comment Actions
Buildworld testing shows this code rarely used after startup.
It's used a lot for I/O to tmpfs mounts, and for sendfile. These aren't exercised by buildworld, though you could try mounting the src and obj trees in tmpfs to cover the first case.
| sys/vm/vm_page.c | ||
|---|---|---|
| 4956 | I meant, shouldn't we check the valid bits and busy the page at pindex + i before the loop, assuming it exists? | |
| sys/vm/vm_page.c | ||
|---|---|---|
| 4956 | I can write it that way. I think "i < ahead" and "m->pindex != pindex + i" are both ways to test for early exit from the inner loop. | |
| 4956 | Currently, if the first non-NULL page is at pindex + 2, we would alloc pindex+1, and then break if page pindex+2 was any_valid or !tryxbusy, and 'after' would be set to 2. If I understand your suggestion properly, it would have us break if page pindex+2 was any_valid or !trybusy, and then 'after' would be set to 1, with nothing allocated. That's a change in behavior of the function. I'm only seeking to improve the performance of the function without changing its behavior. Is this a desirable behavior change? | |
| sys/vm/vm_page.c | ||
|---|---|---|
| 4956 | To be clear, I think the previous version of this patch had a bug: it could potentially busy a resident page that's far beyond the end of the range designated by after. I think this version is right, earlier I had just missed how the m->pindex != pindex + i check ensures that we only busy the page at pindex + ahead. It also catches an early break from the inner loop. | |
| sys/vm/vm_page.c | ||
|---|---|---|
| 4977 | We have not reset the iterator. | |