[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] Speed of drawPolyline on JDK11
From:       Laurent_Bourgès <bourges.laurent () gmail ! com>
Date:       2018-11-27 11:25:33
Message-ID: CAKjRUT58UJ+Ri_ANyH9p_MxVZE+irQFFqp3oLwV6i3Puv-QOkA () mail ! gmail ! com
[Download RAW message or body]

Hi Phil,

> I proposed 2 small fixes for jdk12... still to be done, as better
> solutions are not yet ready and I am completely overbusy on testing Java
> Sorting algorithms.
>
> Maybe I will give up fixing the native renderer, for these reasons:
> - stdlib qsort implementation and performance (bentley qsort 93 or merge
> sort on linux) is platform dependent, so it may take time to validate its
> behaviour (small vs large arrays, not really random, closer to almost
> sorted data)
>
>
> I don't think we should use anyC++  stdlib:: code here if that is what you
> mean.
>

No I proposed to use stdlib (C) qsort(... cmp) as it is done in the same
class to sort initially edges, i.e. see my previous diff => ~1s time.
I prefer focusing on integrating Marlin nonAA (like jfx) to get rid of this
native code, than fixing C code, except such simple change.


>
> Any small improvements we can make by having a better sort in our sources
> would
> be better.
> If they aren't ready for 12, no problem. 13 is around the corner ..
>

Yes, I know, but if you have time, I can propose a very simple Marlin patch
to improve a bit its sorting performance in such extreme case: ~0.5s to
0.22s !


> - AA rendering is now the common case where the Marlin renderer is active
> & fast enough.
>
> Would you accept enabling antialiasing by default in jdk12 ?
> Up to now, RenderingHint.ANTIALIASING_DEFAULT means ANTIALIASING_OFF.
>
>
> That is a big change and I do not think we should do it.
>

Yes it is a one-liner but it can have large impacts, so I agree it should
be discussed in CSR ... so maybe in 13 ?

Cheers,
Laurent



Several users faced performance issues with the native renderer (hidpi,
complex shapes, clipping dashed shapes) where the Marlin renderer performs
far better.

Finally I can now focus on a trivial Marlin patch to tune its sort
algorithm selection...

Cheers,
Laurent

Le jeu. 22 nov. 2018 Ã  00:04, Sergey Bylokhov <Sergey.Bylokhov@oracle.com>
a écrit :

> Hi, Laurent.
>
> I do not think that we should postpone it to the next version.
> Just send the changes when they are ready, if the review fails
> to take place at the right time, then the fix will be
> moved to the jdk13.
>
> On 20/11/2018 00:28, Laurent Bourgès wrote:
> > As OpenJDK12 RDP1 is coming soon, I propose this plan:
> > - integrate this basic fix in ShapeSpanIterator.c code to use stdlib
> sort (mergesort on linux)
> > - integrate a very simple patch in Marlin renderer to disable insertion
> sort for large arrays: 0.5s to 0.25s, few LOC
> > - postpone my changes to Marlin sort & Marlin nonAA renderer integration
> in OpenJDK 13
> >
> > Will you have time to review 2 small patchs on time ?
>
>

[Attachment #3 (text/html)]

<div dir="ltr">Hi Phil,<div class="gmail_quote"><blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div \
text="#000000" bgcolor="#FFFFFF"><blockquote type="cite"><div dir="auto"><div \
dir="auto">I proposed 2 small fixes for jdk12... still to  be done, as better \
solutions are not yet ready and I am  completely overbusy on testing Java Sorting \
algorithms.</div>  <div dir="auto"><br>
        </div>
        <div dir="auto">Maybe I will give up fixing the native renderer,
          for these reasons:</div>
        <div dir="auto">- stdlib qsort implementation and performance
          (bentley qsort 93 or merge sort on linux) is platform
          dependent, so it may take time to validate its behaviour
          (small vs large arrays, not really random, closer to almost
          sorted data)</div>
      </div>
    </blockquote>
    <br>
    I don&#39;t think we should use anyC++   stdlib:: code here if that is
    what you mean.<br></div></blockquote><div><br></div><div>No I proposed to use \
stdlib (C) qsort(... cmp) as it is done in the same class to sort initially edges, \
i.e. see my previous diff =&gt; ~1s time.</div><div>I prefer focusing on integrating \
Marlin nonAA (like jfx) to get rid of this native code, than fixing C code, except \
such simple change.<br></div><div>  </div><blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div \
text="#000000" bgcolor="#FFFFFF">  <br>
    Any small improvements we can make by having a better sort in our
    sources would<br>
    be better.<br>
    If they aren&#39;t ready for 12, no problem. 13 is around the corner \
..<br></div></blockquote><div><br></div><div>Yes, I know, but if you have time, I can \
propose a very simple Marlin patch to improve a bit its sorting performance in such \
extreme case: ~0.5s to 0.22s !<br></div><div>  </div><blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div \
text="#000000" bgcolor="#FFFFFF">  
    <blockquote type="cite">
      <div dir="auto">
        <div dir="auto">- AA rendering is now the common case where the
          Marlin renderer is active &amp; fast enough.</div>
        <div dir="auto"><br>
        </div>
        <div dir="auto">Would you accept enabling antialiasing by
          default in jdk12 ?</div>
        <div dir="auto">Up to now, RenderingHint.ANTIALIASING_DEFAULT
          means  <span style="font-family:sans-serif">ANTIALIASING_</span>OFF.</div>
      </div>
    </blockquote>
    <br>
    That is a big change and I do not think we should do \
it.<br></div></blockquote><div><br></div><div>Yes it is a one-liner but it can have \
large impacts, so I agree it should be discussed in CSR ... so maybe in 13 \
?</div><div><br></div><div>Cheers,</div><div>Laurent<br></div><div text="#000000" \
bgcolor="#FFFFFF">  <br>  <blockquote type="cite">
      <div dir="auto">
        <div dir="auto"><br>
        </div>
        <div dir="auto">Several users faced performance issues with the
          native renderer (hidpi, complex shapes, clipping dashed
          shapes) where the Marlin renderer performs far better.</div>
        <div dir="auto"><br>
        </div>
        <div dir="auto">Finally I can now focus on a trivial Marlin
          patch to tune its sort algorithm selection...</div>
        <div dir="auto"><br>
        </div>
        <div dir="auto">Cheers,</div>
        <div dir="auto">Laurent</div>
        <div dir="auto"><br>
          <div class="gmail_quote" dir="auto">
            <div dir="ltr">Le jeu. 22 nov. 2018 Ã  00:04, Sergey Bylokhov
              &lt;<a href="mailto:Sergey.Bylokhov@oracle.com" \
target="_blank">Sergey.Bylokhov@oracle.com</a>&gt;  a écrit  :<br>
            </div>
            <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex">Hi,  Laurent.<br>
              <br>
              I do not think that we should postpone it to the next
              version.<br>
              Just send the changes when they are ready, if the review
              fails<br>
              to take place at the right time, then the fix will be<br>
              moved to the jdk13.<br>
              <br>
              On 20/11/2018 00:28, Laurent Bourgès wrote:<br>
              &gt; As OpenJDK12 RDP1 is coming soon, I propose this
              plan:<br>
              &gt; - integrate this basic fix in ShapeSpanIterator.c
              code to use stdlib sort (mergesort on linux)<br>
              &gt; - integrate a very simple patch in Marlin renderer to
              disable insertion sort for large arrays: 0.5s to 0.25s,
              few LOC<br>
              &gt; - postpone my changes to Marlin sort &amp; Marlin
              nonAA renderer integration in OpenJDK 13<br>
              &gt; <br>
              &gt; Will you have time to review 2 small patchs on time ?<br>
              <br>
            </blockquote>
          </div>
        </div>
      </div>
    </blockquote><br></div></div></div>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic