RFR: 8189809: Large performance regression in Swing text layout.

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8189809: Large performance regression in Swing text layout.

Phil Race
Bug: https://bugs.openjdk.java.net/browse/JDK-8189809
webrev: http://cr.openjdk.java.net/~prr/8189809/

This partially addresses a slow-down in Swing.

Swing now usually calls Font.getStringBounds() instead of
FontMetrics().stringWidth for text measurement.

The latter has a fast-path for latin-only simple text.

The former does not .. and at a minimum uses GlyphVector.

This fix provides a similar fast path and for direct calls to these
methods (micro benchmarks)
that is latin text they are now very similar .. at least for typical
strings.

In this fix as well as making this change in the font code, I updated
Swing to more
be a little more efficient.

Before this fix I always saw Swing  coming in with a char[] .. then
constructing a String from it.
Then it passes it to the font measurement code, which then decomposes
the string back into a char[]
If Swing directly uses the API that takes a char[] then we save some
overhead.


It does not get back all of the loss by Swing since

1) Swing has other overhead elsewhere - it seems

2) Swing is making 90% of these calls for a single char.
This could be from where Swing naively tries to add up char advances
itself to
get a string advance.

     We may have to come back to that later. Perhaps with new public API.

-phil.


Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8189809: Large performance regression in Swing text layout.

Prahalad kumar Narayanan
Hello Phil

The change looks good to me.

I found two interesting changes in your webrev.
. The move to use FontDesignMetrics to compute width should be better than generating GlyphVector for the same.
. Besides, the double conversion from chars[] to String and vice versa has been avoided as well.

Thank you
Have a good day

Prahalad

-----Original Message-----
From: Phil Race
Sent: Friday, December 08, 2017 2:24 AM
To: 2d-dev; [hidden email]
Subject: [OpenJDK 2D-Dev] RFR: 8189809: Large performance regression in Swing text layout.

Bug: https://bugs.openjdk.java.net/browse/JDK-8189809
webrev: http://cr.openjdk.java.net/~prr/8189809/

This partially addresses a slow-down in Swing.

Swing now usually calls Font.getStringBounds() instead of FontMetrics().stringWidth for text measurement.

The latter has a fast-path for latin-only simple text.

The former does not .. and at a minimum uses GlyphVector.

This fix provides a similar fast path and for direct calls to these methods (micro benchmarks) that is latin text they are now very similar .. at least for typical strings.

In this fix as well as making this change in the font code, I updated Swing to more be a little more efficient.

Before this fix I always saw Swing  coming in with a char[] .. then constructing a String from it.
Then it passes it to the font measurement code, which then decomposes the string back into a char[] If Swing directly uses the API that takes a char[] then we save some overhead.


It does not get back all of the loss by Swing since

1) Swing has other overhead elsewhere - it seems

2) Swing is making 90% of these calls for a single char.
This could be from where Swing naively tries to add up char advances itself to get a string advance.

     We may have to come back to that later. Perhaps with new public API.

-phil.


Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8189809: Large performance regression in Swing text layout.

Sergey Bylokhov
In reply to this post by Phil Race
Hi, Phil.
The new code will calculate the logical bounds this way:
     541 new Rectangle2D.Float(0f, ascent, width, height);
But previously it was:
     2613 new Rectangle2D.Float(0, -tl.getAscent(),   tl.getAdvance(),
     2614                       tl.getAscent() + tl.getDescent() +
     2615                       tl.getLeading());

Note that the second parameter is negative, is it intentionally changed?


On 07/12/2017 12:54, Phil Race wrote:

> Bug: https://bugs.openjdk.java.net/browse/JDK-8189809
> webrev: http://cr.openjdk.java.net/~prr/8189809/
>
> This partially addresses a slow-down in Swing.
>
> Swing now usually calls Font.getStringBounds() instead of
> FontMetrics().stringWidth for text measurement.
>
> The latter has a fast-path for latin-only simple text.
>
> The former does not .. and at a minimum uses GlyphVector.
>
> This fix provides a similar fast path and for direct calls to these
> methods (micro benchmarks)
> that is latin text they are now very similar .. at least for typical
> strings.
>
> In this fix as well as making this change in the font code, I updated
> Swing to more
> be a little more efficient.
>
> Before this fix I always saw Swing  coming in with a char[] .. then
> constructing a String from it.
> Then it passes it to the font measurement code, which then decomposes
> the string back into a char[]
> If Swing directly uses the API that takes a char[] then we save some
> overhead.
>
>
> It does not get back all of the loss by Swing since
>
> 1) Swing has other overhead elsewhere - it seems
>
> 2) Swing is making 90% of these calls for a single char.
> This could be from where Swing naively tries to add up char advances
> itself to
> get a string advance.
>
>      We may have to come back to that later. Perhaps with new public API.
>
> -phil.
>
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8189809: Large performance regression in Swing text layout.

Phil Race
Yes, it should  be negative since we want to use this as the y origin of the rectangle
relative to the baseline. I am using it correctly in the height calculation.
40         float height = ascent + descent + leading;
 541         return new Rectangle2D.Float(0f, ascent, width, height);

http://cr.openjdk.java.net/~prr/8189809.1/

-phil.

On 12/11/2017 02:12 PM, Sergey Bylokhov wrote:
Hi, Phil.
The new code will calculate the logical bounds this way:
    541 new Rectangle2D.Float(0f, ascent, width, height);
But previously it was:
    2613 new Rectangle2D.Float(0, -tl.getAscent(),   tl.getAdvance(),
    2614                       tl.getAscent() + tl.getDescent() +
    2615                       tl.getLeading());

Note that the second parameter is negative, is it intentionally changed?


On 07/12/2017 12:54, Phil Race wrote:
Bug: https://bugs.openjdk.java.net/browse/JDK-8189809
webrev: http://cr.openjdk.java.net/~prr/8189809/

This partially addresses a slow-down in Swing.

Swing now usually calls Font.getStringBounds() instead of FontMetrics().stringWidth for text measurement.

The latter has a fast-path for latin-only simple text.

The former does not .. and at a minimum uses GlyphVector.

This fix provides a similar fast path and for direct calls to these methods (micro benchmarks)
that is latin text they are now very similar .. at least for typical strings.

In this fix as well as making this change in the font code, I updated Swing to more
be a little more efficient.

Before this fix I always saw Swing  coming in with a char[] .. then constructing a String from it.
Then it passes it to the font measurement code, which then decomposes the string back into a char[]
If Swing directly uses the API that takes a char[] then we save some overhead.


It does not get back all of the loss by Swing since

1) Swing has other overhead elsewhere - it seems

2) Swing is making 90% of these calls for a single char.
This could be from where Swing naively tries to add up char advances itself to
get a string advance.

     We may have to come back to that later. Perhaps with new public API.

-phil.





Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8189809: Large performance regression in Swing text layout.

Sergey Bylokhov
Looks fine.

On 11/12/2017 14:39, Phil Race wrote:

> Yes, it should  be negative since we want to use this as the y origin of
> the rectangle
> relative to the baseline. I am using it correctly in the height calculation.
>
> 40 float height = ascent + descent + leading;
> 541 return new Rectangle2D.Float(0f, ascent, width, height);
>
>
> http://cr.openjdk.java.net/~prr/8189809.1/
>
> -phil.
>
> On 12/11/2017 02:12 PM, Sergey Bylokhov wrote:
>> Hi, Phil.
>> The new code will calculate the logical bounds this way:
>>     541 new Rectangle2D.Float(0f, ascent, width, height);
>> But previously it was:
>>     2613 new Rectangle2D.Float(0, -tl.getAscent(), tl.getAdvance(),
>>     2614                       tl.getAscent() + tl.getDescent() +
>>     2615                       tl.getLeading());
>>
>> Note that the second parameter is negative, is it intentionally changed?
>>
>>
>> On 07/12/2017 12:54, Phil Race wrote:
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8189809
>>> webrev: http://cr.openjdk.java.net/~prr/8189809/
>>>
>>> This partially addresses a slow-down in Swing.
>>>
>>> Swing now usually calls Font.getStringBounds() instead of
>>> FontMetrics().stringWidth for text measurement.
>>>
>>> The latter has a fast-path for latin-only simple text.
>>>
>>> The former does not .. and at a minimum uses GlyphVector.
>>>
>>> This fix provides a similar fast path and for direct calls to these
>>> methods (micro benchmarks)
>>> that is latin text they are now very similar .. at least for typical
>>> strings.
>>>
>>> In this fix as well as making this change in the font code, I updated
>>> Swing to more
>>> be a little more efficient.
>>>
>>> Before this fix I always saw Swing  coming in with a char[] .. then
>>> constructing a String from it.
>>> Then it passes it to the font measurement code, which then decomposes
>>> the string back into a char[]
>>> If Swing directly uses the API that takes a char[] then we save some
>>> overhead.
>>>
>>>
>>> It does not get back all of the loss by Swing since
>>>
>>> 1) Swing has other overhead elsewhere - it seems
>>>
>>> 2) Swing is making 90% of these calls for a single char.
>>> This could be from where Swing naively tries to add up char advances
>>> itself to
>>> get a string advance.
>>>
>>>      We may have to come back to that later. Perhaps with new public
>>> API.
>>>
>>> -phil.
>>>
>>>
>>
>>
>


--
Best regards, Sergey.