Thomas, Looks good for me. But please wait for someone from core-libs team.
PS: Could you also fix a typeo at 79, 51, 53? s/initialized/initialization/ 51 * Heap allocated during initialization - one entry per fd -Dmitry On 2016-03-10 10:59, Thomas Stüfe wrote: > Hi, > > may I have a review of this new iteration for this fix? > > Thank you! > > Thomas > > On Thu, Mar 3, 2016 at 5:26 PM, Thomas Stüfe <thomas.stu...@gmail.com> > wrote: > >> Hi all, >> >> https://bugs.openjdk.java.net/browse/JDK-8150460 >> >> thanks to all who took the time to review the first version of this fix! >> >> This is the new version: >> >> >> http://cr.openjdk.java.net/~stuefe/webrevs/8150460-linux_close-fdTable/webrev.02/webrev/ >> >> I reworked the fix, trying to add in all the input I got: This fix uses a >> simple one-dimensional array, preallocated at startup, for low-value file >> descriptors. Like the code did before. Only for large values of file >> descriptors it switches to an overflow table, organized as two dimensional >> sparse array of fixed sized slabs, which are allocated on demand. Only the >> overflow table is protected by a lock. >> >> For 99% of all cases we will be using the plain simple fdTable structure >> as before. Only for unusually large file descriptor values we will be using >> this overflow table. >> >> Memory footprint is kept low: for small values of RLIMIT_NOFILE, we will >> only allocate as much space as we need. Only if file descriptor values get >> large, memory is allocated in the overflow table. >> >> Note that I avoided the proposed double-checked locking solution: I find >> it too risky in this place and also unnecessary. When calling getFdEntry(), >> we will be executing a blocking IO operation afterwards, flanked by two >> mutex locks (in startOp and endOp). So, I do not think the third mutex lock >> in getFdEntry will add much, especially since it is only used in case of >> larger file descriptor values. >> >> I also added the fix to bsd_close.c and aix_close.c. I do not like this >> code triplication. I briefly played around with unifying this code, but >> this is more difficult than it seems: implementations subtly differ between >> the three platforms, and solaris implementation is completely different. It >> may be a worthwhile cleanup, but that would be a separate issue. >> >> I did some artificial tests to check how the code does with many and large >> file descriptor values, all seemed to work well. I also ran java/net jtreg >> tests on Linux and AIX. >> >> Kind Regards, Thomas >> >> -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.