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

[Dmitry Samersoff]

Christoph,

> Dmitry, I think you are referring to an outdated version of the
> webrev, the current one is this:

Yes. Sorry!

You may consider a bit different approach to save memory:

Allocate multiple baseTables for different ranges of fd's with
plain array of 32 * (fdEntry_t*) for simple case.

i.e. if (fd < 32)
         do plain array lookup

     if (fd < N1)
         do two steps lookup in baseTable1

     if (fd < N2)
         do two steps lookup in baseTable2

     ...

-Dmitry



On 2016-03-01 13:47, Langer, Christoph wrote:
> Hi Dmitry, Thomas,
> 
> Dmitry, I think you are referring to an outdated version of the
> webrev, the current one is this: 
> http://cr.openjdk.java.net/~stuefe/webrevs/8150460-linux_close-fdTable/webrev.01/webrev/
>
>  However, I agree - the lock should probably not be taken every time
> but only in the case where we find the entry table was not yet
> allocated.
> 
> So, maybe getFdEntry should always do this: entryTable =
> fdTable[rootArrayIndex]; // no matter if rootArrayIndex is 0
> 
> Then check if entryTable is NULL and if yes then enter a guarded
> section which does the allocation and before that checks if another
> thread did it already.
> 
> Also I'm wondering if the entryArrayMask and the rootArrayMask should
> be calculated once in the init() function and stored in a static
> field? Because right now it is calculated every time getFdEntry() is
> called and I don't think this would be optimized by inlining...
> 
> Best regards Christoph
> 
> -----Original Message----- From: core-libs-dev
> [mailto:core-libs-dev-bounces at openjdk.java.net] On Behalf Of Dmitry
> Samersoff Sent: Dienstag, 1. März 2016 11:20 To: Thomas Stüfe
> <thomas.stuefe at gmail.com>; Java Core Libs
> <core-libs-dev at openjdk.java.net> Subject: Re: RFR(s): 8150460:
> (linux|bsd|aix)_close.c: file descriptor table may become large or
> may not work at all
> 
> Thomas,
> 
> Sorry for being later.
> 
> I'm not sure we should take a lock at ll. 131 for each fdTable
> lookup.
> 
> As soon as we never deallocate fdTable[base_index] it's safe to try
> to return value first and then take a slow path (take a lock and
> check fdTable[base_index] again)
> 
> -Dmitry
> 
> 
> On 2016-02-24 20:30, 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.
>> 
>> 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.
>> 
>> Thank you and 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.