On 1/03/2016 7:27 PM, Thomas Stüfe wrote:
Ping...
Could I have reviewer and a sponsor, please?
You don't need a sponsor for this JDK change - you are a Committer. :)
Cheers,
David
Thanks you!
Thomas
On Thu, Feb 25, 2016 at 5:51 PM, Thomas Stüfe <thomas.stu...@gmail.com>
wrote:
Hi Roger,
thank you for the review!
New webrev:
http://cr.openjdk.java.net/~stuefe/webrevs/8150460-linux_close-fdTable/webrev.01/webrev/
Please find my comments inline.
On Wed, Feb 24, 2016 at 8:28 PM, Roger Riggs <roger.ri...@oracle.com>
wrote:
Hi Thomas,
On 2/24/2016 12:30 PM, Thomas Stüfe wrote:
Hi all,
please take a look at this proposed fix.
The bug: https://bugs.openjdk.java.net/browse/JDK-8150460
The Webrev:
http://cr.openjdk.java.net/~stuefe/webrevs/8150460-linux_close-fdTable/webrev.00/webrev/
Basically, the file descriptor table implemented in linux_close.c may not
work for RLIMIT_NO_FILE=infinite or may grow very large (I saw a 50MB
table) for high values for RLIMIT_NO_FILE. Please see details in the bug
description.
The proposed solution is to implement the file descriptor table not as
plain array, but as a twodimensional sparse array, which grows on demand.
This keeps the memory footprint small and fixes the corner cases
described
in the bug description.
Please note that the implemented solution is kept simple, at the cost of
somewhat higher (some kb) memory footprint for low values of
RLIMIT_NO_FILE.
This can be optimized, if we even think it is worth the trouble.
Please also note that the proposed implementation now uses a mutex lock
for
every call to getFdEntry() - I do not think this matters, as this is all
in
preparation for an IO system call, which are usually way more expensive
than a pthread mutex. But again, this could be optimized.
I would suggest preallocating the index[0] array and then skip the mutex
for that case.
That would give the same as current performance.
I did this.
And I'd suggest a different hi/low split of the indexes to reduce the
size of pre-allocated memory.
Most processes are going to use a lot fewer than 16384 fd's. How about
2048?
I did this too. Now I calculate the split point based on RLIMIT_NO_FILE.
For small values of RLIMIT_NO_FILE
(<64K), I effectivly fall back to a one-dimensional array by making the
first level table a size 1. For larger values,
multiple second level tables, each 64K size, will be allocated on demand
(save for the first one which is preallocated).
I have my doubts about needing to cover fd's up to the full range of 32
bits.
Can the RLIMIT_NO_FILE be used too parametrize the allocation of the
first level table?
I did this.
Interesting note, I found nowhere in the Posix specs a mentioning that
socked descriptors have to be handed out
sequentially and therefore cannot be larger than RLIMIT_NO_FILE. But in
reality on all operating systems file descriptors
seem to be [0, RLIMIT_NO_FILE).
Not specific to your change but it would nice to see consistency between
libnio and libnet on
the name of the sigWakeup/INTERRUPT_SIGNAL constant.
I agree, but this is out of the scope of this bug fix.
This is an implementation proposal for Linux; the same code found its way
to BSD and AIX. Should you approve of this fix, I will modify those files
too.
yes please.
$.02, Roger
Thanks, Thomas
Thank you and Kind Regards, Thomas
