Hi Thomas,
When returning the address of the fdentry, the style using &fdTable[fd]
is preferred over
the implicit pointer arithmetic (as it was in the previous version).
Occurs in all 3 deltas:
src/java.base/*/native/libnet/*_close.c:
+ result = fdTable + fd;
and
+ result = fdOverflowTable[rootindex] + slabindex;
The rest looks fine.
Thanks, Roger
On 3/10/2016 7:59 AM, Thomas Stüfe wrote:
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.
