java.nio.Bits.reserveMemory uses a lock, calls System.gc, and is generally bad code...

886 views
Skip to first unread message

Kevin Burton

unread,
Jul 15, 2013, 6:02:09 PM7/15/13
to mechanica...@googlegroups.com
It looks like this function is called for every directMemory allocation:

Here's a stack trace from a failed memory allocation because I'm setting MAX_DIRECT_MEMORY

at java.nio.Bits.reserveMemory(Bits.java:658)
at java.nio.DirectByteBuffer.<init>(DirectByteBuffer.java:123)
at java.nio.ByteBuffer.allocateDirect(ByteBuffer.java:306)

... there are a couple issues with the below code which makes it pretty ugly (and used OFTEN in direct memory applications):

1.  ALL memory access requires a lock.  That's evil if you're allocating small chunks.

2.  The code to change the reserved memory counters is duplicated twice.  This is a great way to introduce bugs.  (how did this even get approved? do they not do code audits or require that commits be approved?)

3.  If you are out of memory we call System.gc... EVIL.  The entire way direct memory is reclaimed via GC is a horrible design.

4.  After GC they sleep 100ms.  What's that about?  Why 100ms?  Why not 1ms?  

... I think I'm just going to dump this and go with using Unsafe and allocate my own memory directly.  At LEAST for smaller allocations.

    static void reserveMemory(long size, int cap) {
        synchronized (Bits.class) {
            if (!memoryLimitSet && VM.isBooted()) {
                maxMemory = VM.maxDirectMemory();
                memoryLimitSet = true;
            }
            // -XX:MaxDirectMemorySize limits the total capacity rather than the
            // actual memory usage, which will differ when buffers are page
            // aligned.
            if (cap <= maxMemory - totalCapacity) {
                reservedMemory += size;
                totalCapacity += cap;
                count++;
                return;
            }
        }

        System.gc();
        try {
            Thread.sleep(100);
        } catch (InterruptedException x) {
            // Restore interrupt status
            Thread.currentThread().interrupt();
        }
        synchronized (Bits.class) {
            if (totalCapacity + cap > maxMemory)
                throw new OutOfMemoryError("Direct buffer memory");
            reservedMemory += size;
            totalCapacity += cap;
            count++;
        }

    }

Michael Barker

unread,
Jul 15, 2013, 6:18:04 PM7/15/13
to Kevin Burton, mechanica...@googlegroups.com
Yep, this code is awful. It cause a performance issue in production
for us. If your use of DirectByteBuffer memory is not fixed and the
rate of allocation exhausts MAX_DIRECT_MEMORY faster than your on-heap
allocation exhausts the Eden space then you will nasty GC stalls
caused by the call to System.gc(). In most applications this is okay
as the new collection will free up the short lived object
DirectByteBuffers which will de-allocate the associated direct memory
via a PhantomReference notification. However, if you are building
low-latency applications that are conservative with object allocation
to avoid New GC stalls then this can easily bite you.

This "feature" of the JDK meant that we had to push one of our vendors
to pre-allocate and pool all of their DirectByteBuffers.

Mike.
> --
> You received this message because you are subscribed to the Google Groups
> "mechanical-sympathy" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to mechanical-symp...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

Rüdiger Möller

unread,
Jul 17, 2013, 4:54:59 AM7/17/13
to mechanica...@googlegroups.com
WTF, I hope Unsafe access stays for a long time :-)
Reply all
Reply to author
Forward
0 new messages