Phil, As promised, I made Marlin 0.9.1 benchmarks on windows & linux to compare d3d, opengl & xrender backends on the same machine: I5 + nvidia 1070. (I will test on mac OS X soon, if needed)
First Marlin 0.9.1 patch works well: no crash or bug with larger tiles 128x64 even if d3d / opengl uses internally 32x32 texture caches. Moreover xrender pipeline is the fastest compared to D3D (40% slower) or opengl (>250% slower) ! I will share later detailed results (plots) but raw data files are available (see linux & win files): https://github.com/bourgesl/bourgesl.github.io/commit/84133d54692affd1d96938a88ac4a041ec0f5e36 As previously mentioned, I increased the RenderQueue buffer (see patchs at the end) to 4M (instead of 32K) and d3d & opengl get boosted. Finally, I increased the cached tile sizes in OGLVertexCache.h and opengl performance achieved xrender performance on linux: --- /tmp/meld-tmpdLFq6S +++ /home/graphics-rasterizer/jdk/client/src/java.desktop/share/native/common/java2d/opengl/OGLVertexCache.h @@ -38,13 +38,13 @@ * Constants that control the size of the texture tile cache used for * mask operations. */ -#define OGLVC_MASK_CACHE_TILE_WIDTH 32 -#define OGLVC_MASK_CACHE_TILE_HEIGHT 32 +#define OGLVC_MASK_CACHE_TILE_WIDTH 128 /* 32 */ +#define OGLVC_MASK_CACHE_TILE_HEIGHT 64 /* 32 */ #define OGLVC_MASK_CACHE_TILE_SIZE \ (OGLVC_MASK_CACHE_TILE_WIDTH * OGLVC_MASK_CACHE_TILE_HEIGHT) -#define OGLVC_MASK_CACHE_WIDTH_IN_TILES 8 -#define OGLVC_MASK_CACHE_HEIGHT_IN_TILES 4 +#define OGLVC_MASK_CACHE_WIDTH_IN_TILES 16 +#define OGLVC_MASK_CACHE_HEIGHT_IN_TILES 8 #define OGLVC_MASK_CACHE_WIDTH_IN_TEXELS \ (OGLVC_MASK_CACHE_TILE_WIDTH * OGLVC_MASK_CACHE_WIDTH_IN_TILES) Thanks to Clemens, that explained me its xrender new approach and it convinced me to tune java2d pipelines. In conclusion, using larger tiles in Marlin 0.9.1 will allow large gains either in d3d & opengl pipelines so I plan to propose these new fixes for jdk11 soon. News: > - I modified opengl queue buffer capacity recently and got 30% better > performance (mapbench) on linux. That could be worth for macOS: will create > another jbs bug > - I will work on upgrading MarlinFX to 0.9.1 soon to provide you a patch > for openjfx 11. > > Who can review such patches & help me on improving java2d pipelines > (discussion, skills) ? > Soory to insist but who could advice me and give me explanations on the RenderQueue & d3d / opengl backends. I read JBS for RenderQueue buffering as I have several questions (asynchronous queue ?) How to auto-tune such buffer capacity ? It seems tricky as too small or too large buffers impacts performance as it depends on the GPU speed (drain the buffer). Is there any design document ? PS: I will once again look into java2d pipelines for tile constants (32) to see if other parts should be updated (TexturePaint ?)... I also need help on testing such patches on many hw platforms to detect regressions (j2dBench, MapBench...) Cheers, Laurent --- /tmp/meld-tmpFaCMrF +++ /home/graphics-rasterizer/jdk/client/src/java.desktop/share/classes/sun/java2d/pipe/RenderQueue.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -25,9 +25,12 @@ package sun.java2d.pipe; +import java.security.AccessController; import java.util.HashSet; import java.util.Set; + import sun.awt.SunToolkit; +import sun.security.action.GetPropertyAction; /** * The RenderQueue class encapsulates a RenderBuffer on which rendering @@ -72,20 +75,34 @@ public abstract class RenderQueue { /** The size of the underlying buffer, in bytes. */ - private static final int BUFFER_SIZE = 32000; + private static final int BUFFER_SIZE; + + static { + BUFFER_SIZE = align(getInteger("sun.java2d.opengl.bufferSize", 4*1024 * 1024, 32 * 1024, 64 * 1024 * 1024), 1024); + System.out.println("RenderQueue: sun.java2d.opengl.bufferSize = "+BUFFER_SIZE); + } /** The underlying buffer for this queue. */ - protected RenderBuffer buf; + protected final RenderBuffer buf; /** * A Set containing hard references to Objects that must stay alive until * the queue has been completely flushed. */ - protected Set<Object> refSet; + protected final Set<Object> refSet; protected RenderQueue() { - refSet = new HashSet<>(); + refSet = new HashSet<Object>(64); // large enough (LBO) ? buf = RenderBuffer.allocate(BUFFER_SIZE); + } + + protected final void clear() { + // reset the buffer position + buf.clear(); + // clear the set of references, since we no longer need them + refSet.clear(); } /** @@ -220,4 +237,35 @@ buf.position(position); flushNow(); } + + + // system property utilities + public static int getInteger(final String key, final int def, + final int min, final int max) + { + final String property = AccessController.doPrivileged( + new GetPropertyAction(key)); + + int value = def; + if (property != null) { + try { + value = Integer.decode(property); + } catch (NumberFormatException e) { + System.out.println("Invalid integer value for " + key + " = " + property); + } + } + + // check for invalid values + if ((value < min) || (value > max)) { + System.out.println("Invalid value for " + key + " = " + value + + "; expected value in range[" + min + ", " + max + "] !"); + value = def; + } + return value; + } + + protected static int align(final int val, final int norm) { + final int ceil = (int)Math.ceil( ((float) val) / norm); + return ceil * norm; + } } --- /tmp/meld-tmpqByAsT +++ /home/graphics-rasterizer/jdk/client/src/java.desktop/share/classes/sun/java2d/opengl/OGLRenderQueue.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -39,17 +39,25 @@ * the queue, thus ensuring that only one thread communicates with the native * OpenGL libraries for the entire process. */ -public class OGLRenderQueue extends RenderQueue { +public final class OGLRenderQueue extends RenderQueue { + final static long FLUSH_DELAY = getInteger("sun.java2d.opengl.flushDelay", 20, 1, 1000); + + static { + System.out.println("RenderQueue: sun.java2d.opengl.flushDelay = "+FLUSH_DELAY); + } private static OGLRenderQueue theInstance; - private final QueueFlusher flusher; + final QueueFlusher flusher; private OGLRenderQueue() { + super(); /* * The thread must be a member of a thread group * which will not get GCed before VM exit. */ - flusher = AccessController.doPrivileged((PrivilegedAction<QueueFlusher>) QueueFlusher::new); + flusher = AccessController.doPrivileged((PrivilegedAction<QueueFlusher>) () -> { + return new QueueFlusher(ThreadGroupUtils.getRootThreadGroup()); + }); } /** @@ -60,6 +68,7 @@ public static synchronized OGLRenderQueue getInstance() { if (theInstance == null) { theInstance = new OGLRenderQueue(); + theInstance.flusher.start(); } return theInstance; } @@ -114,9 +123,10 @@ * Returns true if the current thread is the OGL QueueFlusher thread. */ public static boolean isQueueFlusherThread() { - return (Thread.currentThread() == getInstance().flusher.thread); - } - + return (Thread.currentThread() == getInstance().flusher); + } + + @Override public void flushNow() { // assert lock.isHeldByCurrentThread(); try { @@ -127,6 +137,7 @@ } } + @Override public void flushAndInvokeNow(Runnable r) { // assert lock.isHeldByCurrentThread(); try { @@ -146,37 +157,32 @@ // process the queue flushBuffer(buf.getAddress(), limit); } - // reset the buffer position - buf.clear(); - // clear the set of references, since we no longer need them - refSet.clear(); - } - - private class QueueFlusher implements Runnable { - private boolean needsFlush; + // reset the queue + clear(); + } + + private final class QueueFlusher extends Thread { + private boolean needsFlush = false; private Runnable task; private Error error; - private final Thread thread; - - public QueueFlusher() { - String name = "Java2D Queue Flusher"; - thread = new Thread(ThreadGroupUtils.getRootThreadGroup(), - this, name, 0, false); - thread.setDaemon(true); - thread.setPriority(Thread.MAX_PRIORITY); - thread.start(); + + QueueFlusher(ThreadGroup threadGroup) { + super(threadGroup, "Java2D Queue Flusher"); + setDaemon(true); + setPriority(Thread.MAX_PRIORITY); } public synchronized void flushNow() { // wake up the flusher needsFlush = true; + notify(); - // wait for flush to complete while (needsFlush) { try { wait(); } catch (InterruptedException e) { + // ignored } } @@ -191,18 +197,20 @@ flushNow(); } + @Override public synchronized void run() { - boolean timedOut = false; + boolean locked = false; + while (true) { + while (!needsFlush) { try { - timedOut = false; /* * Wait until we're woken up with a flushNow() call, * or the timeout period elapses (so that we can * flush the queue periodically). */ - wait(100); + wait(FLUSH_DELAY); /* * We will automatically flush the queue if the * following conditions apply: @@ -211,35 +219,42 @@ * - there is something in the queue to flush * Otherwise, just continue (we'll flush eventually). */ - if (!needsFlush && (timedOut = tryLock())) { + if (!needsFlush && (locked = tryLock())) { if (buf.position() > 0) { needsFlush = true; } else { + locked = false; unlock(); } } } catch (InterruptedException e) { + // ignored } } + // locked by either this thread (see locked flag) or by waiting thread: + // TODO: check lock is always acquired when needsFlush = true ? try { // reset the throwable state error = null; + // flush the buffer now flushBuffer(); + // if there's a task, invoke that now as well if (task != null) { task.run(); + task = null; } } catch (Error e) { error = e; - } catch (Exception x) { + } catch (Exception ex) { System.err.println("exception in QueueFlusher:"); - x.printStackTrace(); + ex.printStackTrace(); } finally { - if (timedOut) { + if (locked) { + locked = false; unlock(); } - task = null; // allow the waiting thread to continue needsFlush = false; notify();