Thomas, > For fd < 65535, I effectively fall back to a plain array lookup by > setting the size of the base table to 1. So, for this case the sparse > array degenerates to a one-dimensional plain array.
It might be good to make it more explicit: just allocate a separate array for values less than 65535 and skip other machinery if nbr_files.rlim_max less than 65536. But it's just a cosmetic, so feel free to leave the code as is. -Dmitry On 2016-03-01 16:33, Thomas Stüfe wrote: > Hi Dmitry, > > On Tue, Mar 1, 2016 at 1:44 PM, Dmitry Samersoff > <dmitry.samers...@oracle.com <mailto:dmitry.samers...@oracle.com>> wrote: > > 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 > > > How does this differ from my approach > in > http://cr.openjdk.java.net/~stuefe/webrevs/8150460-linux_close-fdTable/webrev.01/webrev/ > ? > > For fd < 65535, I effectively fall back to a plain array lookup by > setting the size of the base table to 1. So, for this case the sparse > array degenerates to a one-dimensional plain array. > > Kind Regards, Thomas > > > > ... > > -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-boun...@openjdk.java.net > <mailto:core-libs-dev-boun...@openjdk.java.net>] On Behalf Of Dmitry > > Samersoff Sent: Dienstag, 1. März 2016 11:20 To: Thomas Stüfe > > <thomas.stu...@gmail.com <mailto:thomas.stu...@gmail.com>>; Java > Core Libs > > <core-libs-dev@openjdk.java.net > <mailto:core-libs-dev@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. > > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
