On Tue, 8 Jun 2021 16:45:12 GMT, Henry Jen <henry...@openjdk.org> wrote:

>> src/java.base/unix/native/libjli/java_md.c line 666:
>> 
>>> 664:         return page_size * pages;
>>> 665:     }
>>> 666: }
>> 
>> Could probably be shortened to something like this:
>> 
>> 
>> size_t pagesize = (size_t)sysconf(_SC_PAGESIZE);
>> return (stack_size + (pagesize - 1)) & ~(pagesize - 1);
>> 
>> 
>> or, if you insist on checking for SIZE_MAX:
>> 
>> 
>> size_t pagesize = (size_t)sysconf(_SC_PAGESIZE);
>> size_t max = SIZE_MAX - pagesize;
>> return stack_size <= max ? (stack_size + (pagesize - 1)) & ~(pagesize - 1) : 
>> max;
>> 
>> 
>> (I see David requested this, so this is fine, though passing SIZE_MAX to 
>> this function will quite likely fail anyway :)
>
> While sysconf(_SC_PAGESIZE) is most likely(if not always) be power of 2, it's 
> not a constant we know for sure here and this is not critical path for 
> performance, thus I didn't take that approach.

My concern was not performance but brevity, especially since you add the same 
function twice. And about using the same logic for aligning up as we do within 
hotspot (see align.hpp). You also mix up size_t and long (signed vs unsigned, 
and potentially different sizes) - there is nothing obvious wrong with that but 
I would at least consistently use size_t here.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4256

Reply via email to