Thank you Dmitry! I will fix the typo before comitting.
Kind Regards, Thomas On Thu, Mar 10, 2016 at 9:19 AM, Dmitry Samersoff < dmitry.samers...@oracle.com> wrote: > 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. >
