(Round 2) RFR(s): 8150460: (linux|bsd|aix)_close.c: file descriptor table may become large or may not work at all

[Thomas Stüfe]

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.samersoff at 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.stuefe at 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.
>