You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@jmeter.apache.org by bu...@apache.org on 2017/02/03 22:33:20 UTC
[Bug 60687] Make GUI more responsive when LoggerPanel gets a lot of
events
https://bz.apache.org/bugzilla/show_bug.cgi?id=60687
Philippe Mouawad <p....@ubik-ingenierie.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |p.mouawad@ubik-ingenierie.c
| |om
--- Comment #1 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
Comment on attachment 34714
--> https://bz.apache.org/bugzilla/attachment.cgi?id=34714
Use a bounded fifo buffer for the log events and update LoggerPanel
periodically instead of directly
It looks good to me.
Thanks
--
You are receiving this mail because:
You are the assignee for the bug.
Re: [Bug 60687] Make GUI more responsive when LoggerPanel gets a lot
of events
Posted by Philippe Mouawad <ph...@gmail.com>.
On Sat, Feb 4, 2017 at 2:02 PM, Vladimir Sitnikov <
sitnikov.vladimir@gmail.com> wrote:
> It does not look good to me.
> I think
> 1) SwingUtilities.invokeLater is missing
>
Why ? to guarantee GUI responsive ?
> 2) .clear() misses to set logChanged=true
>
I don't understand why
> 3) if (logChanged) should be expressed as if (!logChanged) {return;}
>
why not
> 4) Do we really need a thread there? How that approach would scale across
> new components (e.g. statistics, etc, etc)?
>
possibly
>
> Vladimir
>
--
Cordialement.
Philippe Mouawad.
Re: [Bug 60687] Make GUI more responsive when LoggerPanel gets a lot
of events
Posted by Vladimir Sitnikov <si...@gmail.com>.
Philippe>But is comparison fair in this case ?
Well, of course it is.
It could make sense to integrate "UI freeze profiling" right into JMeter
code so we could identify those freezes and tame them.
For instance: take an automatic thread dump in case UI thread spends more
than 500ms when processing a request (or something like that).
If we could capture UI latencies somehow, then we could plot them and
compare them (e.g. validate how different fixes impact/improve actual
JMeter use).
Back to the comparison, let me just cite the the conclusions for you (pay
attention to the second sentence of the first paragraph):
According to the measurements, replaceRange + append works faster than
setText except for the case when text is fully replaced. *In 100% replace
case setText is on par with replaceRange + append (the results are within
error bounds).*
For small appends there's a win in terms of response time and memory
allocation.
Vladimir
Re: [Bug 60687] Make GUI more responsive when LoggerPanel gets a lot
of events
Posted by Philippe Mouawad <ph...@gmail.com>.
But is comparison fair in this case ?
Thanks
On Tue, Feb 7, 2017 at 10:54 PM, Vladimir Sitnikov <
sitnikov.vladimir@gmail.com> wrote:
> Philippe>@Vladimir, Sorry if my question is stupid (I may not have fully
> understood), but why percentiles are only on append and not on setText ?
>
> setText replaces the whole text.
> There's no way to "setText just 10% of the text".
>
> On contrary, replace can trim first 10%, then append can append some more
> text.
>
> Vladimir
>
--
Cordialement.
Philippe Mouawad.
Re: [Bug 60687] Make GUI more responsive when LoggerPanel gets a lot
of events
Posted by Vladimir Sitnikov <si...@gmail.com>.
Philippe>@Vladimir, Sorry if my question is stupid (I may not have fully
understood), but why percentiles are only on append and not on setText ?
setText replaces the whole text.
There's no way to "setText just 10% of the text".
On contrary, replace can trim first 10%, then append can append some more
text.
Vladimir
Re: [Bug 60687] Make GUI more responsive when LoggerPanel gets a lot
of events
Posted by Philippe Mouawad <ph...@gmail.com>.
On Wednesday, February 8, 2017, Felix Schumacher <
felix.schumacher@internetallee.de> wrote:
> Am 07.02.2017 22:46, schrieb Philippe Mouawad:
>
>> Hi Vladimir, Felix
>>
>> Thanks for this.
>> @Vladimir, Sorry if my question is stupid (I may not have fully
>> understood), but why percentiles are only on append and not on setText ?
>>
>> @Felix, could you commit your patches ? I'm currently using a lot JMeter
>> these days, having them would let me test them within my everyday work.
>>
>
> I will do it this evening, but if you want to do it earlier, feel free.
it's your code I wouldn't be comfortable to commit it :)
I'll wait, thx
>
> Felix
>
>
>> Thanks
>>
>>
>> Regards
>> Philippe
>>
>> On Sat, Feb 4, 2017 at 6:39 PM, Vladimir Sitnikov <
>> sitnikov.vladimir@gmail.com> wrote:
>>
>> Felix>I looked at that, too. There are two options, one is the String (as
>>> implemented) the other is a RDocument, which didn't look like it would
>>> support it easily. But you might find a find, so look at it.
>>>
>>> Here's my benchmark for setText vs replaceRange+append
>>> https://gist.github.com/vlsi/63a1d4732f027ce70229af830ce20648
>>>
>>> It looks like it makes sense to use replaceRange+append.
>>>
>>> Felix>Formats are not threadsafe, so I put it inside the lock.
>>>
>>> Should we use ThreadLocals for formats?
>>> It seems to be a trivial change, yet it would increase scalability.
>>>
>>> Vladimir
>>>
>>>
--
Cordialement.
Philippe Mouawad.
Re: [Bug 60687] Make GUI more responsive when LoggerPanel gets a lot
of events
Posted by Felix Schumacher <fe...@internetallee.de>.
Am 07.02.2017 22:46, schrieb Philippe Mouawad:
> Hi Vladimir, Felix
>
> Thanks for this.
> @Vladimir, Sorry if my question is stupid (I may not have fully
> understood), but why percentiles are only on append and not on setText
> ?
>
> @Felix, could you commit your patches ? I'm currently using a lot
> JMeter
> these days, having them would let me test them within my everyday work.
I will do it this evening, but if you want to do it earlier, feel free.
Felix
>
> Thanks
>
>
> Regards
> Philippe
>
> On Sat, Feb 4, 2017 at 6:39 PM, Vladimir Sitnikov <
> sitnikov.vladimir@gmail.com> wrote:
>
>> Felix>I looked at that, too. There are two options, one is the String
>> (as
>> implemented) the other is a RDocument, which didn't look like it would
>> support it easily. But you might find a find, so look at it.
>>
>> Here's my benchmark for setText vs replaceRange+append
>> https://gist.github.com/vlsi/63a1d4732f027ce70229af830ce20648
>>
>> It looks like it makes sense to use replaceRange+append.
>>
>> Felix>Formats are not threadsafe, so I put it inside the lock.
>>
>> Should we use ThreadLocals for formats?
>> It seems to be a trivial change, yet it would increase scalability.
>>
>> Vladimir
>>
Re: [Bug 60687] Make GUI more responsive when LoggerPanel gets a lot
of events
Posted by Philippe Mouawad <ph...@gmail.com>.
Hi Vladimir, Felix
Thanks for this.
@Vladimir, Sorry if my question is stupid (I may not have fully
understood), but why percentiles are only on append and not on setText ?
@Felix, could you commit your patches ? I'm currently using a lot JMeter
these days, having them would let me test them within my everyday work.
Thanks
Regards
Philippe
On Sat, Feb 4, 2017 at 6:39 PM, Vladimir Sitnikov <
sitnikov.vladimir@gmail.com> wrote:
> Felix>I looked at that, too. There are two options, one is the String (as
> implemented) the other is a RDocument, which didn't look like it would
> support it easily. But you might find a find, so look at it.
>
> Here's my benchmark for setText vs replaceRange+append
> https://gist.github.com/vlsi/63a1d4732f027ce70229af830ce20648
>
> It looks like it makes sense to use replaceRange+append.
>
> Felix>Formats are not threadsafe, so I put it inside the lock.
>
> Should we use ThreadLocals for formats?
> It seems to be a trivial change, yet it would increase scalability.
>
> Vladimir
>
--
Cordialement.
Philippe Mouawad.
Re: [Bug 60687] Make GUI more responsive when LoggerPanel gets a lot of events
Posted by Felix Schumacher <fe...@internetallee.de>.
Am 4. Februar 2017 18:39:34 MEZ schrieb Vladimir Sitnikov <si...@gmail.com>:
>Felix>I looked at that, too. There are two options, one is the String
>(as
>implemented) the other is a RDocument, which didn't look like it would
>support it easily. But you might find a find, so look at it.
>
>Here's my benchmark for setText vs replaceRange+append
>https://gist.github.com/vlsi/63a1d4732f027ce70229af830ce20648
>
>It looks like it makes sense to use replaceRange+append.
Looks like a nice option, would you like to implement those changes?
>
>Felix>Formats are not threadsafe, so I put it inside the lock.
>
>Should we use ThreadLocals for formats?
>It seems to be a trivial change, yet it would increase scalability.
This is not needed anymore with slf4j :)
Felix
>
>Vladimir
Re: [Bug 60687] Make GUI more responsive when LoggerPanel gets a lot
of events
Posted by Vladimir Sitnikov <si...@gmail.com>.
Felix>I looked at that, too. There are two options, one is the String (as
implemented) the other is a RDocument, which didn't look like it would
support it easily. But you might find a find, so look at it.
Here's my benchmark for setText vs replaceRange+append
https://gist.github.com/vlsi/63a1d4732f027ce70229af830ce20648
It looks like it makes sense to use replaceRange+append.
Felix>Formats are not threadsafe, so I put it inside the lock.
Should we use ThreadLocals for formats?
It seems to be a trivial change, yet it would increase scalability.
Vladimir
Re: [Bug 60687] Make GUI more responsive when LoggerPanel gets a lot
of events
Posted by Felix Schumacher <fe...@internetallee.de>.
Am 04.02.2017 um 16:00 schrieb Philippe Mouawad:
> On Sat, Feb 4, 2017 at 3:52 PM, Vladimir Sitnikov <
> sitnikov.vladimir@gmail.com> wrote:
>
>> Felix> The timer lives in the awt thread, so we need no invokeLater. Or do
>> I miss
>> Felix> something?
>>
>> I thought it was java.util.Timer.
>> I did not notice that was javax.swing.Timer. swing timer is fine.
>>
>> Philippe>I agree. The only reason I would see for calling invokeLater would
>> maybe to
>> Philippe>avoid freezing UI if the call takes more than some limit
>>
>> invokeLater is not required provided the timer is swing timer.
>>
> I agree it's not required but this might be interesting to consider.
> http://stackoverflow.com/questions/15556646/why-are-there-two-timer-classes-in-javaone-under-javax-swing-one-under-java-ut
>
>> I wonder if we can rework log panel into "append" calls instead of "set
>> text from scratch".
>>
> I remember I had tried something around that without success but if you
> have an idea .
I looked at that, too. There are two options, one is the String (as
implemented) the other is a RDocument, which didn't look like it would
support it easily. But you might find a find, so look at it.
>
>> Philippe>For me there is only 1 Timer thread no ?
>>
>> I'm not sure if we can reuse that swing timer to aggregate events form
>> listeners (e.g. statistics listeners, etc).
>>
>> Felix>The component is set to an empty text
>>
>> Oh, I did not notice that. On the other hand, it looks awkward.
>> I find "buffer.clear(); modified=true;" easier to reason about since that
>> results in a single place where the textarea gets updated.
>>
>>
>> PS. Are you sure "events.clear()" is thread-safe?
>>
> I think indeed there might be a problem here
Done.
>
>> PPS. Can we move format.format(logEvent) out of syncrhonized(events) block?
Formats are not threadsafe, so I put it inside the lock.
Felix
>>
>> Vladimir
>>
>
>
Re: [Bug 60687] Make GUI more responsive when LoggerPanel gets a lot
of events
Posted by Philippe Mouawad <ph...@gmail.com>.
On Sat, Feb 4, 2017 at 3:52 PM, Vladimir Sitnikov <
sitnikov.vladimir@gmail.com> wrote:
> Felix> The timer lives in the awt thread, so we need no invokeLater. Or do
> I miss
> Felix> something?
>
> I thought it was java.util.Timer.
> I did not notice that was javax.swing.Timer. swing timer is fine.
>
> Philippe>I agree. The only reason I would see for calling invokeLater would
> maybe to
> Philippe>avoid freezing UI if the call takes more than some limit
>
> invokeLater is not required provided the timer is swing timer.
>
I agree it's not required but this might be interesting to consider.
http://stackoverflow.com/questions/15556646/why-are-there-two-timer-classes-in-javaone-under-javax-swing-one-under-java-ut
>
> I wonder if we can rework log panel into "append" calls instead of "set
> text from scratch".
>
I remember I had tried something around that without success but if you
have an idea .
>
> Philippe>For me there is only 1 Timer thread no ?
>
> I'm not sure if we can reuse that swing timer to aggregate events form
> listeners (e.g. statistics listeners, etc).
>
> Felix>The component is set to an empty text
>
> Oh, I did not notice that. On the other hand, it looks awkward.
> I find "buffer.clear(); modified=true;" easier to reason about since that
> results in a single place where the textarea gets updated.
>
>
> PS. Are you sure "events.clear()" is thread-safe?
>
I think indeed there might be a problem here
>
> PPS. Can we move format.format(logEvent) out of syncrhonized(events) block?
>
> Vladimir
>
--
Cordialement.
Philippe Mouawad.
Re: [Bug 60687] Make GUI more responsive when LoggerPanel gets a lot
of events
Posted by Vladimir Sitnikov <si...@gmail.com>.
Felix> The timer lives in the awt thread, so we need no invokeLater. Or do
I miss
Felix> something?
I thought it was java.util.Timer.
I did not notice that was javax.swing.Timer. swing timer is fine.
Philippe>I agree. The only reason I would see for calling invokeLater would
maybe to
Philippe>avoid freezing UI if the call takes more than some limit
invokeLater is not required provided the timer is swing timer.
I wonder if we can rework log panel into "append" calls instead of "set
text from scratch".
Philippe>For me there is only 1 Timer thread no ?
I'm not sure if we can reuse that swing timer to aggregate events form
listeners (e.g. statistics listeners, etc).
Felix>The component is set to an empty text
Oh, I did not notice that. On the other hand, it looks awkward.
I find "buffer.clear(); modified=true;" easier to reason about since that
results in a single place where the textarea gets updated.
PS. Are you sure "events.clear()" is thread-safe?
PPS. Can we move format.format(logEvent) out of syncrhonized(events) block?
Vladimir
Re: [Bug 60687] Make GUI more responsive when LoggerPanel gets a lot
of events
Posted by Philippe Mouawad <ph...@gmail.com>.
On Sat, Feb 4, 2017 at 3:06 PM, Felix Schumacher <
felix.schumacher@internetallee.de> wrote:
>
>
> Am 4. Februar 2017 14:02:58 MEZ schrieb Vladimir Sitnikov <
> sitnikov.vladimir@gmail.com>:
> >It does not look good to me.
> >I think
> >1) SwingUtilities.invokeLater is missing
>
> The timer lives in the awt thread, so we need no invokeLater. Or do I miss
> something?
>
I agree. The only reason I would see for calling invokeLater would maybe to
avoid freezing UI if the call takes more than some limit.
>
> >2) .clear() misses to set logChanged=true
>
> The component is set to an empty text, so logChanged can be left alone, or
> even set to false. Don't you think?
>
> >3) if (logChanged) should be expressed as if (!logChanged) {return;}
>
> Probably.
>
> >4) Do we really need a thread there? How that approach would scale
> >across
> >new components (e.g. statistics, etc, etc)?
>
> You mean the timer thread?
>
I think so but I don't understand the comment. For me there is only 1 Timer
thread no ?
>
> Felix
>
> >
> >Vladimir
>
--
Cordialement.
Philippe Mouawad.
Re: [Bug 60687] Make GUI more responsive when LoggerPanel gets a lot of events
Posted by Felix Schumacher <fe...@internetallee.de>.
Am 4. Februar 2017 14:02:58 MEZ schrieb Vladimir Sitnikov <si...@gmail.com>:
>It does not look good to me.
>I think
>1) SwingUtilities.invokeLater is missing
The timer lives in the awt thread, so we need no invokeLater. Or do I miss something?
>2) .clear() misses to set logChanged=true
The component is set to an empty text, so logChanged can be left alone, or even set to false. Don't you think?
>3) if (logChanged) should be expressed as if (!logChanged) {return;}
Probably.
>4) Do we really need a thread there? How that approach would scale
>across
>new components (e.g. statistics, etc, etc)?
You mean the timer thread?
Felix
>
>Vladimir
Re: [Bug 60687] Make GUI more responsive when LoggerPanel gets a lot
of events
Posted by Vladimir Sitnikov <si...@gmail.com>.
It does not look good to me.
I think
1) SwingUtilities.invokeLater is missing
2) .clear() misses to set logChanged=true
3) if (logChanged) should be expressed as if (!logChanged) {return;}
4) Do we really need a thread there? How that approach would scale across
new components (e.g. statistics, etc, etc)?
Vladimir