[OpenJDK Rasterizer] Marlin #4

[Laurent Bourgès]

Hi Jim,

Sorry I sent the message partially edited by mistake.

> The point is that the hard failure is a condition of when we need more
than we can provide, not when we "already have" more than we can provide.
needSize should cause the hard failure, not the current size.  And if
needSize is going to cause the hard failure then why use >= instead of > -
we can supply MAX_INT if that is what is "needed".
>
> Also, there is a potential bug right now that curSize could be < MAX_INT,
but needSize could be > MAX_INT, but the test at line 205 and the
adjustment at line 212 will decide that we can return with MAX_INT being
the answer - even though that is less than what we really need.
>
> It is just much more straightforward to have the tests be:
>
> - if we are planning to allocate more than MAX
>    (that isn't going to work, so...)
>    - then if we really *need* more than MAX => exception
>    - otherwise just allocate MAX
>
> Another way to code this would be to first clip against MAX as being a
hard limit and then throw an exception if the planned amount "isn't enough":
>
> if (size > MAX_INT) size = MAX_INT;
> if (size < needSize) exception("We couldn't allocate enough");
> return size;

I like it your approach but will try rewriting it to use cascaded if blocks
that saves 1 condition if size <= MAX_INT (common path).

>> Now I only check 1/8th scanlines ie once per pixel row as it is roughly
>> "continuous" (maybe false positive) and it implies the very first row is
>> always non RLE encoded.
>
>
> Is this intentional?  Or can the heuristic be done at the beginning so
that the first pixel row also benefits from it?

Yes it saves 7/8 metric evaluations and it seems to me complicated to
inverse the logic. In my early tests, I had several flags to know if
blkFlags were partially used (to reset them). I like as it is now.

I could improve the heuristic to infer if the average (crossing width) is
centered on a tile to maximize the possibility a complete 32 block to be
skipped in the stroking case... to be discussed later.

>>     We can enable runtime-logging later on if we find a decent way to
>>     have it be low impact when not runtime-enabled...

>> However I would prefer having enableLogs = true if the renderer is in
>> verbose mode: "  -Dsun.java2d.renderer.verbose=true".
>>
>> So it would display the startup informations in this case (including
>> tuning settings ...) and it would allow gathering Marlin statistics (if
>> doStats = true).
>
> I could see that, but perhaps it should be controlled by a
marlin-specific attribute?

Like ... marlin.verbose ? Or simply marlin.enableLogs ?

> In any case, this is very simple to add later once we get this
integrated.  I just wanted to be sure that initial integration took the
path of least resistance to silencing it.

Agreed. It works very well but if we want to tune some settings (tile size,
pixel size, TL vs CLQ), it is very useful to log them and possibly get
statistics.

>
>> Moreover, I should refactor j.u.l.Logger usage by PlatformLogger (TODO)
>> and we should rename all Marlin System properties to use the prefix
>> "sun.java2d.marlin..."
>
>
> I would support that for marlin-specific properties...

Ok, I propose to change the String prefix in MarlinProperties and fix the
MarlinRE.logSettings ()

Regards,
Laurent
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/graphics-rasterizer-dev/attachments/20151123/cbf87d60/attachment.html>