-
Enhancement
-
Resolution: Fixed
-
P4
-
16
-
b23
There are some problems with how PretouchTask calculates the chunk size and number of chunks to be touched.
In pretouch(), the number of chunks, used to determine the number of workers, is
size_t num_chunks = MAX2((size_t)1, total_bytes / MAX2(PretouchTask::chunk_size(), page_size));
This is rounding down total_bytes/{chunk,page}_size. It should probably be rounding up. Rounding up would also remove the need for the max with 1. Maybe there should be a check for an empty range, doing nothing in that case.
In the work function, the chunk size is
size_t const actual_chunk_size = MAX2(chunk_size(), _page_size);
This value is also calculated in pretouch() to determine num_chunks (see below). However, the calculations may be inconsistent. The calculation in pretouch() is done without regard for the conditional adjustment of _page_size for UseTransparentHugePages.
It would be better to calculate the chunk size once in pretouch (with the page size adjustment also done there), and pass both the chunk size and the adjusted page size to the PretouchTask constructor for recording in the task.
[I think the only bad effect is that we might spin up more or fewer pretouch tasks than ideal, so calling this an enhancement rather than a bug.]
[I noticed these issues during theJDK-8252221 refactoring, but they were pre-existing and didn't impact the refactoring.]
In pretouch(), the number of chunks, used to determine the number of workers, is
size_t num_chunks = MAX2((size_t)1, total_bytes / MAX2(PretouchTask::chunk_size(), page_size));
This is rounding down total_bytes/{chunk,page}_size. It should probably be rounding up. Rounding up would also remove the need for the max with 1. Maybe there should be a check for an empty range, doing nothing in that case.
In the work function, the chunk size is
size_t const actual_chunk_size = MAX2(chunk_size(), _page_size);
This value is also calculated in pretouch() to determine num_chunks (see below). However, the calculations may be inconsistent. The calculation in pretouch() is done without regard for the conditional adjustment of _page_size for UseTransparentHugePages.
It would be better to calculate the chunk size once in pretouch (with the page size adjustment also done there), and pass both the chunk size and the adjusted page size to the PretouchTask constructor for recording in the task.
[I think the only bad effect is that we might spin up more or fewer pretouch tasks than ideal, so calling this an enhancement rather than a bug.]
[I noticed these issues during the
- relates to
-
JDK-8259380 Correct pretouch chunk size to cap with actual page size
- Resolved
-
JDK-8275426 PretouchTask num_chunks calculation can overflow
- Closed
-
JDK-8252221 Use multiple workers for Parallel GC pre-touching
- Resolved