Hi everyone, I registered autumn of last year for a bugfix(Bug JDK-8229902). I have a working bugfix, but was asked to investigate the issue a bit deeper in order to find an appropriate course of action. I wasnt able to put in enough time before the end-of-year and beginning-of-year madness at my dayjobs, so sorry for the slow pace. Now, I was able to put in some more time and I feel like I am a step further.
So here's my two cents: Like already explained, the bug is caused by the conditional flush()-call in the freespace-function (this is called to make sure that there is enough space in the buffer, before something is put into it). The unconditional flushBuffer()-call works fine, however. You can fix the bug by setting m_autoflush to false or commenting out the flush()-call in freespace. (file: /modules/javafx.web/src/main/native/Source/WebCore/platform/graphics/java/RenderingQueue.cpp) Since both functions contain the word "flush", I feel the urge to explain: The RenderingQueue is basically a LinkedList of Bytearrays. the freespace-function checks if there is enough space in the current bytearray for whatever you want to put in it. If there is not enough space, a new Bytearray is declared and set as current, so that what you put into the buffer gets put into the new bytearray. However, this new bytearray is NOT part of the LinkedList on the java side of things, but exists only in c++. So here is what the two functions are doing: - flushBuffer "flushes" the current c++ bytearray into the java LinkedList and declares a new current c++-Bytearray. After this, you have a new, empty c++ Bytearray, and the LinkedList on the java side contains one more Bytearray (the old current c++ array) - flush() "flushes" the whole LinkedList of bytearrays. It decodes all its Bytearrays. After this, you have a new, empty java LinkedList<BufferData>. (in file: modules/javafx.web/src/main/java/com/sun/webkit/graphics /WCRenderQueue.java, BufferData is a helper class around a ByteArray, its a bit confusing, flushBuffer adds a new BufferData to the list, but only on the next flushBuffer-call, the bytebuffer of the bufferdata gets set to the bytearray, and can therefore be decoded by flush()) Now there is still some mess here, (does it work to put in Glyphs? to me it looks like they never make it into the bytearray) there are flushbuffer calls directly before freespace-calls, so clearly space and time is wasted. (~30% time on my setup, still havent checked how much memory can be saved) I see two ways of tackling this beast: 1. clearly the naming is confusing and has lead to mistakes in using the RenderingQueue from the outside. I could maybe make the naming a bit better and explain the two functions better in the comments. Then fix or take out the unnecessary/inefficient flush/flushBuffer-calls, just from a time and (memory)space point of view, the bug AND related inefficencies are perfectly solvable this way. 2. solve the problem "once and for all": The way I see it, todays buffer-solution is unnecessary complicated. Wouldnt it be nicer, if you could just put things into the buffer and let the RenderingQueue worry about the details(is there enough space etc.)? Same thing when you want the Queue to decode. The fact that the current bytebuffer has to be "flushed" into the LinkedList before decoding is a special technique of the RenderingQueue. Of course, you should know about it, if you are messing around inside RenderingQueue. But when you are just using a RenderingQueue, I think you should be freed from the resposibility of micromanaging RenderingQueue. I will need to put in some more time for either solution. This is not just a bugfix for the specific bug. My first approach was to fix the bug with minimal changes (just "flipping" one bit from true to false), now I would prefer to fix the underlying problem in a more substantial way. And if I'm doing that, I would prefer strategy (2.), rather than (1.) Even tough (2.) will not cause alot of code change, it will significantly change RenderingQueues behavior as seen from the outside, rather than just fix its wrong or inefficient behavior. Of course I will still need to present the solution in a git commit in order for you people to decide if thats a good idea/strategy. What do you guys (or just Kevin ?) think? You want me to put my time into a more comprehensive strategy or is my time better spent fixing specific problems and leave the bigger changes to people with more experience and knowledge? kind regards Dani Kurmann
