Re: [AMB] AMB Digest, Vol 3, Issue 8
by Marek Timko
Hi,
Yes, it's better to copy value, timestamp, sequence and zone.
I'm getting busy, but if I will have time I can check try your thread safe
queue.
Marek.
On Jun 27, 2014 12:17 AM, <amb-request(a)lists.01.org> wrote:
> Send AMB mailing list submissions to
> amb(a)lists.01.org
>
> To subscribe or unsubscribe via the World Wide Web, visit
> https://lists.01.org/mailman/listinfo/amb
> or, via email, send a message with subject or body 'help' to
> amb-request(a)lists.01.org
>
> You can reach the person managing the list at
> amb-owner(a)lists.01.org
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of AMB digest..."
>
>
> Today's Topics:
>
> 1. Re: AMB Digest, Vol 3, Issue 5 (Kevron Rees)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Thu, 26 Jun 2014 15:16:59 -0700
> From: Kevron Rees <tripzero.kev(a)gmail.com>
> To: Marek Timko <marek.timko1(a)gmail.com>, "amb(a)lists.01.org"
> <amb(a)lists.01.org>
> Subject: Re: [AMB] AMB Digest, Vol 3, Issue 5
> Message-ID:
> <CAAOZRYWzEaoO-0EqouQSgaXiciYAVtxf=
> R9O0UO1CuUuR-nCjQ(a)mail.gmail.com>
> Content-Type: text/plain; charset="utf-8"
>
> I'm playing with a thread safe queue in my private fork to make
> updateProperty() thread safe. It doesn't appear to have a negative impact
> on performance. Please try at your convenience.
>
>
> https://github.com/tripzero/automotive-message-broker/commit/d0f063514d40...
>
> This change however, makes updateProperty less deterministic. The mainloop
> will execute updateProperty instead of the source plugins directly.
> Meaning that updateProperty() will be called whenever the mainloop gets
> around to calling it. While this might be a slight disadvantage, it opens
> up the possibility to do priority-level filtering later on. Based upon
> configuration, different queues can have different execution priorities
> handled by the mainloop.
>
> It's also possible we can have a thread-safe updateProperty() and a
> non-thread safe one. The non-thread safe one would be more deterministic.
>
> On Thu, Jun 26, 2014 at 1:27 PM, Marek Timko <marek.timko1(a)gmail.com>
> wrote:
>
> > 1. I'm not sue if all plugins have/will have socket fd. You will need to
> > refactor can bus.
> > 2. and 3. You have to somehow wake up main thread. Timer is not a good
> > solution. In 3a you have to maintain AbstractPropertyType lifetime and
> > copying it is quite expensive.
> >
> > I think that it's a better to make routing engine thread safe. And crash
> > was in dbus plugin.
> > Btw instead of the delete mValue and assigning the new pointer you can
> use
> > mValue->setValue(). You will save some CPU time. That's why I did not get
> > the crash in my modified code.(is boost::any thread safe???).
> >
>
> setValue won't copy the timestamp, it creates a new timestamp. It also
> doesn't copy sequence. The rest should be the same. Maybe its' faster to
> copy the timestamp, value and sequence rather than delete the old value?
>
> -Kevron
>
>
> > Marek.
> > On Jun 25, 2014 7:30 AM, "Kevron Rees" <tripzero.kev(a)gmail.com> wrote:
> >
> >>
> >>
> >>
> >> On Tue, Jun 24, 2014 at 9:34 PM, Marek Timko <marek.timko1(a)gmail.com>
> >> wrote:
> >>
> >>> That is the problem. My plugins are calling routing engine from their
> >>> own threads.
> >>>
> >>> How you can ensure that nobody will do it?
> >>>
> >> We should at least add it to the documentation for
> >> AbstractRoutingEngine...
> >>
> >>
> >>> Routing engine e has to be thread safe.
> >>>
> >>> How I can call routing engine from ambd main thread, If I have
> separate
> >>> thread for receiving CAN frames???
> >>>
> >>
> >> We can solve this in 3 ways.
> >>
> >> 1. Don't use threads. Use GIOChannel and g_io_add_watch() to watch for
> >> activity on the CAN socket fd. We do this in gpsnmea and other plugins.
> >> 2. Use a thread safe queue and marshal the data back to the main thread
> >> before calling routingEngine->updateProperty()
> >> 3. If we anticipate that threads will be used a lot in source plugins
> (we
> >> do use threads in several plugins, be we always do 2), we can make
> >> updateProperty() thread safe by doing (2) internally inside the routing
> >> engine logic:
> >> a. updateProperty() simply appends an AbstractPropertyType to a thread
> >> safe queue
> >> b. We put a watch on the thread safe queue so when it has contents, the
> >> mainloop will wake up and pop the data off and process it
> >>
> >>
> >>
> >>
> >>> Marek
> >>> On Jun 25, 2014 2:35 AM, <amb-request(a)lists.01.org> wrote:
> >>>
> >>>> Send AMB mailing list submissions to
> >>>> amb(a)lists.01.org
> >>>>
> >>>> To subscribe or unsubscribe via the World Wide Web, visit
> >>>> https://lists.01.org/mailman/listinfo/amb
> >>>> or, via email, send a message with subject or body 'help' to
> >>>> amb-request(a)lists.01.org
> >>>>
> >>>> You can reach the person managing the list at
> >>>> amb-owner(a)lists.01.org
> >>>>
> >>>> When replying, please edit your Subject line so it is more specific
> >>>> than "Re: Contents of AMB digest..."
> >>>>
> >>>>
> >>>> Today's Topics:
> >>>>
> >>>> 1. Re: AMB Performance (Rees, Kevron)
> >>>> 2. Re: AMB Performance (Marek Timko)
> >>>> 3. Re: AMB Performance (Kevron Rees)
> >>>>
> >>>>
> >>>> ----------------------------------------------------------------------
> >>>>
> >>>> Message: 1
> >>>> Date: Tue, 24 Jun 2014 13:22:15 -0700
> >>>> From: "Rees, Kevron" <kevron.m.rees(a)intel.com>
> >>>> To: Marek Timko <marek.timko1(a)gmail.com>
> >>>> Cc: amb(a)lists.01.org
> >>>> Subject: Re: [AMB] AMB Performance
> >>>> Message-ID:
> >>>> <
> >>>> CAFW5wYZEhm8D9jR7p3Ysk6UJReien9-4jrhRcyZc9hf0G-V2dA(a)mail.gmail.com>
> >>>> Content-Type: text/plain; charset=UTF-8
> >>>>
> >>>> On Tue, Jun 24, 2014 at 11:21 AM, Rees, Kevron <
> kevron.m.rees(a)intel.com>
> >>>> wrote:
> >>>> > On Tue, Jun 24, 2014 at 10:11 AM, Marek Timko <
> marek.timko1(a)gmail.com>
> >>>> wrote:
> >>>> >> Hi,
> >>>> >>
> >>>> >> I checked the latest code and I see 2 issues that can cause a
> crash.
> >>>> >> Both related to DBus signaler.
> >>>> >> 1. You have to protect queue with mutex,
> >>>> >> because timer may be called from different thread than thread in
> >>>> which you
> >>>> >> are inserting into queue.
> >>>> >
> >>>> > Right now it all happens in the same thread. When would it not be
> in
> >>>> > the same thread?
> >>>> >
> >>>> >> 2. AbstractType::setValue does delete mValue and set the new
> >>>> pointer. From
> >>>> >> dbus signaler you are accessing mValue. I can reproduce crash in <
> >>>> 50ms when
> >>>> >> changing property every 1 ms.
> >>>> >>
> >>>> >
> >>>> > Good catch. I'll look into it.
> >>>>
> >>>> What's the best way to reproduce it. I've got the following config
> >>>> and I'm not able to reproduce it... It could be that my local changes
> >>>> are obfuscating reproduction though...
> >>>>
> >>>> {
> >>>> "sources" : [
> >>>> {
> >>>> "name" : "ExampleSouce",
> >>>> "path" :
> >>>> "/usr/lib/automotive-message-broker/examplesourceplugin.so",
> >>>> "delay" : "0"
> >>>> }
> >>>> ],
> >>>> "sinks": [
> >>>> {
> >>>> "name" : "DBusSink",
> >>>> "path" :
> >>>> "/usr/lib/automotive-message-broker/dbussinkplugin.so",
> >>>> "frequency" : "60"
> >>>> }
> >>>>
> >>>> ]
> >>>> }
> >>>>
> >>>>
> >>>> >
> >>>> >> I did some performance improvement, but I haven't been able to
> >>>> compare
> >>>> >> because of the crash of the original code :-)
> >>>> >>
> >>>> >> Marek
> >>>> >>
> >>>> >> On Jun 11, 2014 9:41 PM, "Marek Timko" <marek.timko1(a)gmail.com>
> >>>> wrote:
> >>>> >>>
> >>>> >>> Just came to my mind: I have a very good experience with Intel
> >>>> VTune ;-)
> >>>> >>> It was on Windows, but it's available also for Linux.
> >>>> >>>
> >>>> >>> https://software.intel.com/en-us/intel-vtune-amplifier-xe
> >>>> >>>
> >>>> >>> It's able to to show CPU usage per function.
> >>>> >>>
> >>>> >>> Marek
> >>>> >>>
> >>>> >>> On Tue, Jun 10, 2014 at 12:47 AM, marek.timko1(a)gmail.com
> >>>> >>> <marek.timko1(a)gmail.com> wrote:
> >>>> >>> >
> >>>> >>> > Hi Kevron,
> >>>> >>> > I see that there is still 8-9% CPU usage in performance
> >>>> >>> > test(https://bugs.tizen.org/jira/browse/TIVI-2623).
> >>>> >>>
> >>>> >>> I haven't pushed some of the more impactful changes to tizen yet.
> >>>> So
> >>>> >>> the most recent test doesn't include those (0.11.809 vs 810).
> >>>> >>>
> >>>> >>> > I suggest to use valgrind with callgrind
> plugin(--tool=callgrind).
> >>>> >>> > QtCreator has a nice support plugin for valgrind.
> >>>> >>> > You can find tho most often called methods(and time consumption)
> >>>> and
> >>>> >>> > improove the most critical then.
> >>>> >>> >
> >>>> >>>
> >>>> >>> Agreed. I do use that tool in QtCreator. It's very nice.
> There's
> >>>> a
> >>>> >>> lot of data to sift through. So far I've only been addressing the
> >>>> >>> biggest pigs I can find, which are typically related to DBus
> usage.
> >>>> >>> But as that becomes less of an issue, I'll have to use the tools
> to
> >>>> >>> find smaller "pigs".
> >>>> >>>
> >>>> >>> > And I still think that you should replace all method calls with
> >>>> >>> > arguments
> >>>> >>> > passed by value with arguments passed by (const) reference. At
> >>>> least
> >>>> >>> > those
> >>>> >>> > under lib folder - it affect all plugins and can be called very
> >>>> often.
> >>>> >>> > For example
> >>>> AbstractRoutingEngine::setProperty(AsyncSetPropertyRequest
> >>>> >>> > request) is called for every signal change, If I remember
> >>>> correctly.
> >>>> >>> > It also should reduce CPU usage. ( Imagine std::string -> there
> is
> >>>> >>> > always at
> >>>> >>> > least one malloc, one memcopy and one free call for each method
> >>>> call
> >>>> >>> > with
> >>>> >>> > arguments passed by value for each std::string argument).
> >>>> >>> >
> >>>> >>>
> >>>> >>> Agreed. Passing by const ref makes sense.
> >>>> >>>
> >>>> >>> > Marek.
> >>>> >>> >
> >>>> >>> > Sent from my mobile
> >>>> >>> >
> >>>> >>> > _______________________________________________
> >>>> >>> > AMB mailing list
> >>>> >>> > AMB(a)lists.01.org
> >>>> >>> > https://lists.01.org/mailman/listinfo/amb
> >>>> >>> >
> >>>>
> >>>>
> >>>> ------------------------------
> >>>>
> >>>> Message: 2
> >>>> Date: Tue, 24 Jun 2014 22:49:49 +0200
> >>>> From: Marek Timko <marek.timko1(a)gmail.com>
> >>>> To: "Rees, Kevron" <kevron.m.rees(a)intel.com>
> >>>> Cc: amb(a)lists.01.org
> >>>> Subject: Re: [AMB] AMB Performance
> >>>> Message-ID:
> >>>> <CADNADdEHBG04qdi3jCR-Bpiu5mj6bvc7Y=2eoJzn=
> >>>> R3rLs0R6g(a)mail.gmail.com>
> >>>> Content-Type: text/plain; charset="utf-8"
> >>>>
> >>>> I used can plugin (source plugin, can be cansimplugin) and canplay
> >>>> utility(
> >>>> from can-utils) to send ~1000 can frames / changes of the VehicleSpeed
> >>>> per
> >>>> second.
> >>>> On Jun 24, 2014 10:22 PM, "Rees, Kevron" <kevron.m.rees(a)intel.com>
> >>>> wrote:
> >>>>
> >>>> > On Tue, Jun 24, 2014 at 11:21 AM, Rees, Kevron <
> >>>> kevron.m.rees(a)intel.com>
> >>>> > wrote:
> >>>> > > On Tue, Jun 24, 2014 at 10:11 AM, Marek Timko <
> >>>> marek.timko1(a)gmail.com>
> >>>> > wrote:
> >>>> > >> Hi,
> >>>> > >>
> >>>> > >> I checked the latest code and I see 2 issues that can cause a
> >>>> crash.
> >>>> > >> Both related to DBus signaler.
> >>>> > >> 1. You have to protect queue with mutex,
> >>>> > >> because timer may be called from different thread than thread in
> >>>> which
> >>>> > you
> >>>> > >> are inserting into queue.
> >>>> > >
> >>>> > > Right now it all happens in the same thread. When would it not be
> >>>> in
> >>>> > > the same thread?
> >>>> > >
> >>>> > >> 2. AbstractType::setValue does delete mValue and set the new
> >>>> pointer.
> >>>> > From
> >>>> > >> dbus signaler you are accessing mValue. I can reproduce crash in
> <
> >>>> 50ms
> >>>> > when
> >>>> > >> changing property every 1 ms.
> >>>> > >>
> >>>> > >
> >>>> > > Good catch. I'll look into it.
> >>>> >
> >>>> > What's the best way to reproduce it. I've got the following config
> >>>> > and I'm not able to reproduce it... It could be that my local
> changes
> >>>> > are obfuscating reproduction though...
> >>>> >
> >>>> > {
> >>>> > "sources" : [
> >>>> > {
> >>>> > "name" : "ExampleSouce",
> >>>> > "path" :
> >>>> > "/usr/lib/automotive-message-broker/examplesourceplugin.so",
> >>>> > "delay" : "0"
> >>>> > }
> >>>> > ],
> >>>> > "sinks": [
> >>>> > {
> >>>> > "name" : "DBusSink",
> >>>> > "path" :
> >>>> > "/usr/lib/automotive-message-broker/dbussinkplugin.so",
> >>>> > "frequency" : "60"
> >>>> > }
> >>>> >
> >>>> > ]
> >>>> > }
> >>>> >
> >>>> >
> >>>> > >
> >>>> > >> I did some performance improvement, but I haven't been able to
> >>>> compare
> >>>> > >> because of the crash of the original code :-)
> >>>> > >>
> >>>> > >> Marek
> >>>> > >>
> >>>> > >> On Jun 11, 2014 9:41 PM, "Marek Timko" <marek.timko1(a)gmail.com>
> >>>> wrote:
> >>>> > >>>
> >>>> > >>> Just came to my mind: I have a very good experience with Intel
> >>>> VTune
> >>>> > ;-)
> >>>> > >>> It was on Windows, but it's available also for Linux.
> >>>> > >>>
> >>>> > >>> https://software.intel.com/en-us/intel-vtune-amplifier-xe
> >>>> > >>>
> >>>> > >>> It's able to to show CPU usage per function.
> >>>> > >>>
> >>>> > >>> Marek
> >>>> > >>>
> >>>> > >>> On Tue, Jun 10, 2014 at 12:47 AM, marek.timko1(a)gmail.com
> >>>> > >>> <marek.timko1(a)gmail.com> wrote:
> >>>> > >>> >
> >>>> > >>> > Hi Kevron,
> >>>> > >>> > I see that there is still 8-9% CPU usage in performance
> >>>> > >>> > test(https://bugs.tizen.org/jira/browse/TIVI-2623).
> >>>> > >>>
> >>>> > >>> I haven't pushed some of the more impactful changes to tizen
> yet.
> >>>> So
> >>>> > >>> the most recent test doesn't include those (0.11.809 vs 810).
> >>>> > >>>
> >>>> > >>> > I suggest to use valgrind with callgrind
> >>>> plugin(--tool=callgrind).
> >>>> > >>> > QtCreator has a nice support plugin for valgrind.
> >>>> > >>> > You can find tho most often called methods(and time
> >>>> consumption) and
> >>>> > >>> > improove the most critical then.
> >>>> > >>> >
> >>>> > >>>
> >>>> > >>> Agreed. I do use that tool in QtCreator. It's very nice.
> >>>> There's a
> >>>> > >>> lot of data to sift through. So far I've only been addressing
> the
> >>>> > >>> biggest pigs I can find, which are typically related to DBus
> >>>> usage.
> >>>> > >>> But as that becomes less of an issue, I'll have to use the tools
> >>>> to
> >>>> > >>> find smaller "pigs".
> >>>> > >>>
> >>>> > >>> > And I still think that you should replace all method calls
> with
> >>>> > >>> > arguments
> >>>> > >>> > passed by value with arguments passed by (const) reference. At
> >>>> least
> >>>> > >>> > those
> >>>> > >>> > under lib folder - it affect all plugins and can be called
> very
> >>>> > often.
> >>>> > >>> > For example
> >>>> > AbstractRoutingEngine::setProperty(AsyncSetPropertyRequest
> >>>> > >>> > request) is called for every signal change, If I remember
> >>>> correctly.
> >>>> > >>> > It also should reduce CPU usage. ( Imagine std::string ->
> there
> >>>> is
> >>>> > >>> > always at
> >>>> > >>> > least one malloc, one memcopy and one free call for each
> method
> >>>> call
> >>>> > >>> > with
> >>>> > >>> > arguments passed by value for each std::string argument).
> >>>> > >>> >
> >>>> > >>>
> >>>> > >>> Agreed. Passing by const ref makes sense.
> >>>> > >>>
> >>>> > >>> > Marek.
> >>>> > >>> >
> >>>> > >>> > Sent from my mobile
> >>>> > >>> >
> >>>> > >>> > _______________________________________________
> >>>> > >>> > AMB mailing list
> >>>> > >>> > AMB(a)lists.01.org
> >>>> > >>> > https://lists.01.org/mailman/listinfo/amb
> >>>> > >>> >
> >>>> >
> >>>> -------------- next part --------------
> >>>> An HTML attachment was scrubbed...
> >>>> URL: <
> >>>>
> http://lists.01.org/pipermail/amb/attachments/20140624/3255cdb2/attachmen...
> >>>> >
> >>>>
> >>>> ------------------------------
> >>>>
> >>>> Message: 3
> >>>> Date: Tue, 24 Jun 2014 17:35:03 -0700
> >>>> From: Kevron Rees <tripzero.kev(a)gmail.com>
> >>>> To: Marek Timko <marek.timko1(a)gmail.com>
> >>>> Cc: amb(a)lists.01.org
> >>>> Subject: Re: [AMB] AMB Performance
> >>>> Message-ID:
> >>>> <
> >>>> CAAOZRYVRBjp6Q6iof109GitxyjHXn2EtB+C-O-jcEBPpketu-g(a)mail.gmail.com>
> >>>> Content-Type: text/plain; charset="utf-8"
> >>>>
> >>>> Delay 0 produces about 300k changes in the example source. I'm not
> >>>> hitting
> >>>> a crash. Signaller uses the abstractproperty. So it shouldn't ever
> >>>> crash
> >>>> unless it was being changed by a different thread at the same time as
> >>>> the
> >>>> signal was being fired...
> >>>>
> >>>> Plugins shouldn't be calling any routing engine calls from different
> >>>> threads. RoutingEngine is not thread safe
> >>>> On Jun 24, 2014 1:50 PM, "Marek Timko" <marek.timko1(a)gmail.com>
> wrote:
> >>>>
> >>>> > I used can plugin (source plugin, can be cansimplugin) and canplay
> >>>> > utility( from can-utils) to send ~1000 can frames / changes of the
> >>>> > VehicleSpeed per second.
> >>>> > On Jun 24, 2014 10:22 PM, "Rees, Kevron" <kevron.m.rees(a)intel.com>
> >>>> wrote:
> >>>> >
> >>>> >> On Tue, Jun 24, 2014 at 11:21 AM, Rees, Kevron <
> >>>> kevron.m.rees(a)intel.com>
> >>>> >> wrote:
> >>>> >> > On Tue, Jun 24, 2014 at 10:11 AM, Marek Timko <
> >>>> marek.timko1(a)gmail.com>
> >>>> >> wrote:
> >>>> >> >> Hi,
> >>>> >> >>
> >>>> >> >> I checked the latest code and I see 2 issues that can cause a
> >>>> crash.
> >>>> >> >> Both related to DBus signaler.
> >>>> >> >> 1. You have to protect queue with mutex,
> >>>> >> >> because timer may be called from different thread than thread in
> >>>> which
> >>>> >> you
> >>>> >> >> are inserting into queue.
> >>>> >> >
> >>>> >> > Right now it all happens in the same thread. When would it not
> be
> >>>> in
> >>>> >> > the same thread?
> >>>> >> >
> >>>> >> >> 2. AbstractType::setValue does delete mValue and set the new
> >>>> pointer.
> >>>> >> From
> >>>> >> >> dbus signaler you are accessing mValue. I can reproduce crash
> in <
> >>>> >> 50ms when
> >>>> >> >> changing property every 1 ms.
> >>>> >> >>
> >>>> >> >
> >>>> >> > Good catch. I'll look into it.
> >>>> >>
> >>>> >> What's the best way to reproduce it. I've got the following config
> >>>> >> and I'm not able to reproduce it... It could be that my local
> >>>> changes
> >>>> >> are obfuscating reproduction though...
> >>>> >>
> >>>> >> {
> >>>> >> "sources" : [
> >>>> >> {
> >>>> >> "name" : "ExampleSouce",
> >>>> >> "path" :
> >>>> >> "/usr/lib/automotive-message-broker/examplesourceplugin.so",
> >>>> >> "delay" : "0"
> >>>> >> }
> >>>> >> ],
> >>>> >> "sinks": [
> >>>> >> {
> >>>> >> "name" : "DBusSink",
> >>>> >> "path" :
> >>>> >> "/usr/lib/automotive-message-broker/dbussinkplugin.so",
> >>>> >> "frequency" : "60"
> >>>> >> }
> >>>> >>
> >>>> >> ]
> >>>> >> }
> >>>> >>
> >>>> >>
> >>>> >> >
> >>>> >> >> I did some performance improvement, but I haven't been able to
> >>>> compare
> >>>> >> >> because of the crash of the original code :-)
> >>>> >> >>
> >>>> >> >> Marek
> >>>> >> >>
> >>>> >> >> On Jun 11, 2014 9:41 PM, "Marek Timko" <marek.timko1(a)gmail.com>
> >>>> wrote:
> >>>> >> >>>
> >>>> >> >>> Just came to my mind: I have a very good experience with Intel
> >>>> VTune
> >>>> >> ;-)
> >>>> >> >>> It was on Windows, but it's available also for Linux.
> >>>> >> >>>
> >>>> >> >>> https://software.intel.com/en-us/intel-vtune-amplifier-xe
> >>>> >> >>>
> >>>> >> >>> It's able to to show CPU usage per function.
> >>>> >> >>>
> >>>> >> >>> Marek
> >>>> >> >>>
> >>>> >> >>> On Tue, Jun 10, 2014 at 12:47 AM, marek.timko1(a)gmail.com
> >>>> >> >>> <marek.timko1(a)gmail.com> wrote:
> >>>> >> >>> >
> >>>> >> >>> > Hi Kevron,
> >>>> >> >>> > I see that there is still 8-9% CPU usage in performance
> >>>> >> >>> > test(https://bugs.tizen.org/jira/browse/TIVI-2623).
> >>>> >> >>>
> >>>> >> >>> I haven't pushed some of the more impactful changes to tizen
> >>>> yet. So
> >>>> >> >>> the most recent test doesn't include those (0.11.809 vs 810).
> >>>> >> >>>
> >>>> >> >>> > I suggest to use valgrind with callgrind
> >>>> plugin(--tool=callgrind).
> >>>> >> >>> > QtCreator has a nice support plugin for valgrind.
> >>>> >> >>> > You can find tho most often called methods(and time
> >>>> consumption) and
> >>>> >> >>> > improove the most critical then.
> >>>> >> >>> >
> >>>> >> >>>
> >>>> >> >>> Agreed. I do use that tool in QtCreator. It's very nice.
> >>>> There's a
> >>>> >> >>> lot of data to sift through. So far I've only been addressing
> >>>> the
> >>>> >> >>> biggest pigs I can find, which are typically related to DBus
> >>>> usage.
> >>>> >> >>> But as that becomes less of an issue, I'll have to use the
> tools
> >>>> to
> >>>> >> >>> find smaller "pigs".
> >>>> >> >>>
> >>>> >> >>> > And I still think that you should replace all method calls
> with
> >>>> >> >>> > arguments
> >>>> >> >>> > passed by value with arguments passed by (const) reference.
> At
> >>>> least
> >>>> >> >>> > those
> >>>> >> >>> > under lib folder - it affect all plugins and can be called
> very
> >>>> >> often.
> >>>> >> >>> > For example
> >>>> >> AbstractRoutingEngine::setProperty(AsyncSetPropertyRequest
> >>>> >> >>> > request) is called for every signal change, If I remember
> >>>> correctly.
> >>>> >> >>> > It also should reduce CPU usage. ( Imagine std::string ->
> >>>> there is
> >>>> >> >>> > always at
> >>>> >> >>> > least one malloc, one memcopy and one free call for each
> >>>> method call
> >>>> >> >>> > with
> >>>> >> >>> > arguments passed by value for each std::string argument).
> >>>> >> >>> >
> >>>> >> >>>
> >>>> >> >>> Agreed. Passing by const ref makes sense.
> >>>> >> >>>
> >>>> >> >>> > Marek.
> >>>> >> >>> >
> >>>> >> >>> > Sent from my mobile
> >>>> >> >>> >
> >>>> >> >>> > _______________________________________________
> >>>> >> >>> > AMB mailing list
> >>>> >> >>> > AMB(a)lists.01.org
> >>>> >> >>> > https://lists.01.org/mailman/listinfo/amb
> >>>> >> >>> >
> >>>> >>
> >>>> >
> >>>> > _______________________________________________
> >>>> > AMB mailing list
> >>>> > AMB(a)lists.01.org
> >>>> > https://lists.01.org/mailman/listinfo/amb
> >>>> >
> >>>> >
> >>>> -------------- next part --------------
> >>>> An HTML attachment was scrubbed...
> >>>> URL: <
> >>>>
> http://lists.01.org/pipermail/amb/attachments/20140624/29eb02ea/attachmen...
> >>>> >
> >>>>
> >>>> ------------------------------
> >>>>
> >>>> Subject: Digest Footer
> >>>>
> >>>> _______________________________________________
> >>>> AMB mailing list
> >>>> AMB(a)lists.01.org
> >>>> https://lists.01.org/mailman/listinfo/amb
> >>>>
> >>>>
> >>>> ------------------------------
> >>>>
> >>>> End of AMB Digest, Vol 3, Issue 5
> >>>> *********************************
> >>>>
> >>>
> >>> _______________________________________________
> >>> AMB mailing list
> >>> AMB(a)lists.01.org
> >>> https://lists.01.org/mailman/listinfo/amb
> >>>
> >>>
> >>
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: <
> http://lists.01.org/pipermail/amb/attachments/20140626/477727e1/attachmen...
> >
>
> ------------------------------
>
> Subject: Digest Footer
>
> _______________________________________________
> AMB mailing list
> AMB(a)lists.01.org
> https://lists.01.org/mailman/listinfo/amb
>
>
> ------------------------------
>
> End of AMB Digest, Vol 3, Issue 8
> *********************************
>
6 years, 5 months
- 1
- 0
Re: [AMB] AMB Digest, Vol 3, Issue 5
by Kevron Rees
I'm playing with a thread safe queue in my private fork to make
updateProperty() thread safe. It doesn't appear to have a negative impact
on performance. Please try at your convenience.
https://github.com/tripzero/automotive-message-broker/commit/d0f063514d40...
This change however, makes updateProperty less deterministic. The mainloop
will execute updateProperty instead of the source plugins directly.
Meaning that updateProperty() will be called whenever the mainloop gets
around to calling it. While this might be a slight disadvantage, it opens
up the possibility to do priority-level filtering later on. Based upon
configuration, different queues can have different execution priorities
handled by the mainloop.
It's also possible we can have a thread-safe updateProperty() and a
non-thread safe one. The non-thread safe one would be more deterministic.
On Thu, Jun 26, 2014 at 1:27 PM, Marek Timko <marek.timko1(a)gmail.com> wrote:
> 1. I'm not sue if all plugins have/will have socket fd. You will need to
> refactor can bus.
> 2. and 3. You have to somehow wake up main thread. Timer is not a good
> solution. In 3a you have to maintain AbstractPropertyType lifetime and
> copying it is quite expensive.
>
> I think that it's a better to make routing engine thread safe. And crash
> was in dbus plugin.
> Btw instead of the delete mValue and assigning the new pointer you can use
> mValue->setValue(). You will save some CPU time. That's why I did not get
> the crash in my modified code.(is boost::any thread safe???).
>
setValue won't copy the timestamp, it creates a new timestamp. It also
doesn't copy sequence. The rest should be the same. Maybe its' faster to
copy the timestamp, value and sequence rather than delete the old value?
-Kevron
> Marek.
> On Jun 25, 2014 7:30 AM, "Kevron Rees" <tripzero.kev(a)gmail.com> wrote:
>
>>
>>
>>
>> On Tue, Jun 24, 2014 at 9:34 PM, Marek Timko <marek.timko1(a)gmail.com>
>> wrote:
>>
>>> That is the problem. My plugins are calling routing engine from their
>>> own threads.
>>>
>>> How you can ensure that nobody will do it?
>>>
>> We should at least add it to the documentation for
>> AbstractRoutingEngine...
>>
>>
>>> Routing engine e has to be thread safe.
>>>
>>> How I can call routing engine from ambd main thread, If I have separate
>>> thread for receiving CAN frames???
>>>
>>
>> We can solve this in 3 ways.
>>
>> 1. Don't use threads. Use GIOChannel and g_io_add_watch() to watch for
>> activity on the CAN socket fd. We do this in gpsnmea and other plugins.
>> 2. Use a thread safe queue and marshal the data back to the main thread
>> before calling routingEngine->updateProperty()
>> 3. If we anticipate that threads will be used a lot in source plugins (we
>> do use threads in several plugins, be we always do 2), we can make
>> updateProperty() thread safe by doing (2) internally inside the routing
>> engine logic:
>> a. updateProperty() simply appends an AbstractPropertyType to a thread
>> safe queue
>> b. We put a watch on the thread safe queue so when it has contents, the
>> mainloop will wake up and pop the data off and process it
>>
>>
>>
>>
>>> Marek
>>> On Jun 25, 2014 2:35 AM, <amb-request(a)lists.01.org> wrote:
>>>
>>>> Send AMB mailing list submissions to
>>>> amb(a)lists.01.org
>>>>
>>>> To subscribe or unsubscribe via the World Wide Web, visit
>>>> https://lists.01.org/mailman/listinfo/amb
>>>> or, via email, send a message with subject or body 'help' to
>>>> amb-request(a)lists.01.org
>>>>
>>>> You can reach the person managing the list at
>>>> amb-owner(a)lists.01.org
>>>>
>>>> When replying, please edit your Subject line so it is more specific
>>>> than "Re: Contents of AMB digest..."
>>>>
>>>>
>>>> Today's Topics:
>>>>
>>>> 1. Re: AMB Performance (Rees, Kevron)
>>>> 2. Re: AMB Performance (Marek Timko)
>>>> 3. Re: AMB Performance (Kevron Rees)
>>>>
>>>>
>>>> ----------------------------------------------------------------------
>>>>
>>>> Message: 1
>>>> Date: Tue, 24 Jun 2014 13:22:15 -0700
>>>> From: "Rees, Kevron" <kevron.m.rees(a)intel.com>
>>>> To: Marek Timko <marek.timko1(a)gmail.com>
>>>> Cc: amb(a)lists.01.org
>>>> Subject: Re: [AMB] AMB Performance
>>>> Message-ID:
>>>> <
>>>> CAFW5wYZEhm8D9jR7p3Ysk6UJReien9-4jrhRcyZc9hf0G-V2dA(a)mail.gmail.com>
>>>> Content-Type: text/plain; charset=UTF-8
>>>>
>>>> On Tue, Jun 24, 2014 at 11:21 AM, Rees, Kevron <kevron.m.rees(a)intel.com>
>>>> wrote:
>>>> > On Tue, Jun 24, 2014 at 10:11 AM, Marek Timko <marek.timko1(a)gmail.com>
>>>> wrote:
>>>> >> Hi,
>>>> >>
>>>> >> I checked the latest code and I see 2 issues that can cause a crash.
>>>> >> Both related to DBus signaler.
>>>> >> 1. You have to protect queue with mutex,
>>>> >> because timer may be called from different thread than thread in
>>>> which you
>>>> >> are inserting into queue.
>>>> >
>>>> > Right now it all happens in the same thread. When would it not be in
>>>> > the same thread?
>>>> >
>>>> >> 2. AbstractType::setValue does delete mValue and set the new
>>>> pointer. From
>>>> >> dbus signaler you are accessing mValue. I can reproduce crash in <
>>>> 50ms when
>>>> >> changing property every 1 ms.
>>>> >>
>>>> >
>>>> > Good catch. I'll look into it.
>>>>
>>>> What's the best way to reproduce it. I've got the following config
>>>> and I'm not able to reproduce it... It could be that my local changes
>>>> are obfuscating reproduction though...
>>>>
>>>> {
>>>> "sources" : [
>>>> {
>>>> "name" : "ExampleSouce",
>>>> "path" :
>>>> "/usr/lib/automotive-message-broker/examplesourceplugin.so",
>>>> "delay" : "0"
>>>> }
>>>> ],
>>>> "sinks": [
>>>> {
>>>> "name" : "DBusSink",
>>>> "path" :
>>>> "/usr/lib/automotive-message-broker/dbussinkplugin.so",
>>>> "frequency" : "60"
>>>> }
>>>>
>>>> ]
>>>> }
>>>>
>>>>
>>>> >
>>>> >> I did some performance improvement, but I haven't been able to
>>>> compare
>>>> >> because of the crash of the original code :-)
>>>> >>
>>>> >> Marek
>>>> >>
>>>> >> On Jun 11, 2014 9:41 PM, "Marek Timko" <marek.timko1(a)gmail.com>
>>>> wrote:
>>>> >>>
>>>> >>> Just came to my mind: I have a very good experience with Intel
>>>> VTune ;-)
>>>> >>> It was on Windows, but it's available also for Linux.
>>>> >>>
>>>> >>> https://software.intel.com/en-us/intel-vtune-amplifier-xe
>>>> >>>
>>>> >>> It's able to to show CPU usage per function.
>>>> >>>
>>>> >>> Marek
>>>> >>>
>>>> >>> On Tue, Jun 10, 2014 at 12:47 AM, marek.timko1(a)gmail.com
>>>> >>> <marek.timko1(a)gmail.com> wrote:
>>>> >>> >
>>>> >>> > Hi Kevron,
>>>> >>> > I see that there is still 8-9% CPU usage in performance
>>>> >>> > test(https://bugs.tizen.org/jira/browse/TIVI-2623).
>>>> >>>
>>>> >>> I haven't pushed some of the more impactful changes to tizen yet.
>>>> So
>>>> >>> the most recent test doesn't include those (0.11.809 vs 810).
>>>> >>>
>>>> >>> > I suggest to use valgrind with callgrind plugin(--tool=callgrind).
>>>> >>> > QtCreator has a nice support plugin for valgrind.
>>>> >>> > You can find tho most often called methods(and time consumption)
>>>> and
>>>> >>> > improove the most critical then.
>>>> >>> >
>>>> >>>
>>>> >>> Agreed. I do use that tool in QtCreator. It's very nice. There's
>>>> a
>>>> >>> lot of data to sift through. So far I've only been addressing the
>>>> >>> biggest pigs I can find, which are typically related to DBus usage.
>>>> >>> But as that becomes less of an issue, I'll have to use the tools to
>>>> >>> find smaller "pigs".
>>>> >>>
>>>> >>> > And I still think that you should replace all method calls with
>>>> >>> > arguments
>>>> >>> > passed by value with arguments passed by (const) reference. At
>>>> least
>>>> >>> > those
>>>> >>> > under lib folder - it affect all plugins and can be called very
>>>> often.
>>>> >>> > For example
>>>> AbstractRoutingEngine::setProperty(AsyncSetPropertyRequest
>>>> >>> > request) is called for every signal change, If I remember
>>>> correctly.
>>>> >>> > It also should reduce CPU usage. ( Imagine std::string -> there is
>>>> >>> > always at
>>>> >>> > least one malloc, one memcopy and one free call for each method
>>>> call
>>>> >>> > with
>>>> >>> > arguments passed by value for each std::string argument).
>>>> >>> >
>>>> >>>
>>>> >>> Agreed. Passing by const ref makes sense.
>>>> >>>
>>>> >>> > Marek.
>>>> >>> >
>>>> >>> > Sent from my mobile
>>>> >>> >
>>>> >>> > _______________________________________________
>>>> >>> > AMB mailing list
>>>> >>> > AMB(a)lists.01.org
>>>> >>> > https://lists.01.org/mailman/listinfo/amb
>>>> >>> >
>>>>
>>>>
>>>> ------------------------------
>>>>
>>>> Message: 2
>>>> Date: Tue, 24 Jun 2014 22:49:49 +0200
>>>> From: Marek Timko <marek.timko1(a)gmail.com>
>>>> To: "Rees, Kevron" <kevron.m.rees(a)intel.com>
>>>> Cc: amb(a)lists.01.org
>>>> Subject: Re: [AMB] AMB Performance
>>>> Message-ID:
>>>> <CADNADdEHBG04qdi3jCR-Bpiu5mj6bvc7Y=2eoJzn=
>>>> R3rLs0R6g(a)mail.gmail.com>
>>>> Content-Type: text/plain; charset="utf-8"
>>>>
>>>> I used can plugin (source plugin, can be cansimplugin) and canplay
>>>> utility(
>>>> from can-utils) to send ~1000 can frames / changes of the VehicleSpeed
>>>> per
>>>> second.
>>>> On Jun 24, 2014 10:22 PM, "Rees, Kevron" <kevron.m.rees(a)intel.com>
>>>> wrote:
>>>>
>>>> > On Tue, Jun 24, 2014 at 11:21 AM, Rees, Kevron <
>>>> kevron.m.rees(a)intel.com>
>>>> > wrote:
>>>> > > On Tue, Jun 24, 2014 at 10:11 AM, Marek Timko <
>>>> marek.timko1(a)gmail.com>
>>>> > wrote:
>>>> > >> Hi,
>>>> > >>
>>>> > >> I checked the latest code and I see 2 issues that can cause a
>>>> crash.
>>>> > >> Both related to DBus signaler.
>>>> > >> 1. You have to protect queue with mutex,
>>>> > >> because timer may be called from different thread than thread in
>>>> which
>>>> > you
>>>> > >> are inserting into queue.
>>>> > >
>>>> > > Right now it all happens in the same thread. When would it not be
>>>> in
>>>> > > the same thread?
>>>> > >
>>>> > >> 2. AbstractType::setValue does delete mValue and set the new
>>>> pointer.
>>>> > From
>>>> > >> dbus signaler you are accessing mValue. I can reproduce crash in <
>>>> 50ms
>>>> > when
>>>> > >> changing property every 1 ms.
>>>> > >>
>>>> > >
>>>> > > Good catch. I'll look into it.
>>>> >
>>>> > What's the best way to reproduce it. I've got the following config
>>>> > and I'm not able to reproduce it... It could be that my local changes
>>>> > are obfuscating reproduction though...
>>>> >
>>>> > {
>>>> > "sources" : [
>>>> > {
>>>> > "name" : "ExampleSouce",
>>>> > "path" :
>>>> > "/usr/lib/automotive-message-broker/examplesourceplugin.so",
>>>> > "delay" : "0"
>>>> > }
>>>> > ],
>>>> > "sinks": [
>>>> > {
>>>> > "name" : "DBusSink",
>>>> > "path" :
>>>> > "/usr/lib/automotive-message-broker/dbussinkplugin.so",
>>>> > "frequency" : "60"
>>>> > }
>>>> >
>>>> > ]
>>>> > }
>>>> >
>>>> >
>>>> > >
>>>> > >> I did some performance improvement, but I haven't been able to
>>>> compare
>>>> > >> because of the crash of the original code :-)
>>>> > >>
>>>> > >> Marek
>>>> > >>
>>>> > >> On Jun 11, 2014 9:41 PM, "Marek Timko" <marek.timko1(a)gmail.com>
>>>> wrote:
>>>> > >>>
>>>> > >>> Just came to my mind: I have a very good experience with Intel
>>>> VTune
>>>> > ;-)
>>>> > >>> It was on Windows, but it's available also for Linux.
>>>> > >>>
>>>> > >>> https://software.intel.com/en-us/intel-vtune-amplifier-xe
>>>> > >>>
>>>> > >>> It's able to to show CPU usage per function.
>>>> > >>>
>>>> > >>> Marek
>>>> > >>>
>>>> > >>> On Tue, Jun 10, 2014 at 12:47 AM, marek.timko1(a)gmail.com
>>>> > >>> <marek.timko1(a)gmail.com> wrote:
>>>> > >>> >
>>>> > >>> > Hi Kevron,
>>>> > >>> > I see that there is still 8-9% CPU usage in performance
>>>> > >>> > test(https://bugs.tizen.org/jira/browse/TIVI-2623).
>>>> > >>>
>>>> > >>> I haven't pushed some of the more impactful changes to tizen yet.
>>>> So
>>>> > >>> the most recent test doesn't include those (0.11.809 vs 810).
>>>> > >>>
>>>> > >>> > I suggest to use valgrind with callgrind
>>>> plugin(--tool=callgrind).
>>>> > >>> > QtCreator has a nice support plugin for valgrind.
>>>> > >>> > You can find tho most often called methods(and time
>>>> consumption) and
>>>> > >>> > improove the most critical then.
>>>> > >>> >
>>>> > >>>
>>>> > >>> Agreed. I do use that tool in QtCreator. It's very nice.
>>>> There's a
>>>> > >>> lot of data to sift through. So far I've only been addressing the
>>>> > >>> biggest pigs I can find, which are typically related to DBus
>>>> usage.
>>>> > >>> But as that becomes less of an issue, I'll have to use the tools
>>>> to
>>>> > >>> find smaller "pigs".
>>>> > >>>
>>>> > >>> > And I still think that you should replace all method calls with
>>>> > >>> > arguments
>>>> > >>> > passed by value with arguments passed by (const) reference. At
>>>> least
>>>> > >>> > those
>>>> > >>> > under lib folder - it affect all plugins and can be called very
>>>> > often.
>>>> > >>> > For example
>>>> > AbstractRoutingEngine::setProperty(AsyncSetPropertyRequest
>>>> > >>> > request) is called for every signal change, If I remember
>>>> correctly.
>>>> > >>> > It also should reduce CPU usage. ( Imagine std::string -> there
>>>> is
>>>> > >>> > always at
>>>> > >>> > least one malloc, one memcopy and one free call for each method
>>>> call
>>>> > >>> > with
>>>> > >>> > arguments passed by value for each std::string argument).
>>>> > >>> >
>>>> > >>>
>>>> > >>> Agreed. Passing by const ref makes sense.
>>>> > >>>
>>>> > >>> > Marek.
>>>> > >>> >
>>>> > >>> > Sent from my mobile
>>>> > >>> >
>>>> > >>> > _______________________________________________
>>>> > >>> > AMB mailing list
>>>> > >>> > AMB(a)lists.01.org
>>>> > >>> > https://lists.01.org/mailman/listinfo/amb
>>>> > >>> >
>>>> >
>>>> -------------- next part --------------
>>>> An HTML attachment was scrubbed...
>>>> URL: <
>>>> http://lists.01.org/pipermail/amb/attachments/20140624/3255cdb2/attachmen...
>>>> >
>>>>
>>>> ------------------------------
>>>>
>>>> Message: 3
>>>> Date: Tue, 24 Jun 2014 17:35:03 -0700
>>>> From: Kevron Rees <tripzero.kev(a)gmail.com>
>>>> To: Marek Timko <marek.timko1(a)gmail.com>
>>>> Cc: amb(a)lists.01.org
>>>> Subject: Re: [AMB] AMB Performance
>>>> Message-ID:
>>>> <
>>>> CAAOZRYVRBjp6Q6iof109GitxyjHXn2EtB+C-O-jcEBPpketu-g(a)mail.gmail.com>
>>>> Content-Type: text/plain; charset="utf-8"
>>>>
>>>> Delay 0 produces about 300k changes in the example source. I'm not
>>>> hitting
>>>> a crash. Signaller uses the abstractproperty. So it shouldn't ever
>>>> crash
>>>> unless it was being changed by a different thread at the same time as
>>>> the
>>>> signal was being fired...
>>>>
>>>> Plugins shouldn't be calling any routing engine calls from different
>>>> threads. RoutingEngine is not thread safe
>>>> On Jun 24, 2014 1:50 PM, "Marek Timko" <marek.timko1(a)gmail.com> wrote:
>>>>
>>>> > I used can plugin (source plugin, can be cansimplugin) and canplay
>>>> > utility( from can-utils) to send ~1000 can frames / changes of the
>>>> > VehicleSpeed per second.
>>>> > On Jun 24, 2014 10:22 PM, "Rees, Kevron" <kevron.m.rees(a)intel.com>
>>>> wrote:
>>>> >
>>>> >> On Tue, Jun 24, 2014 at 11:21 AM, Rees, Kevron <
>>>> kevron.m.rees(a)intel.com>
>>>> >> wrote:
>>>> >> > On Tue, Jun 24, 2014 at 10:11 AM, Marek Timko <
>>>> marek.timko1(a)gmail.com>
>>>> >> wrote:
>>>> >> >> Hi,
>>>> >> >>
>>>> >> >> I checked the latest code and I see 2 issues that can cause a
>>>> crash.
>>>> >> >> Both related to DBus signaler.
>>>> >> >> 1. You have to protect queue with mutex,
>>>> >> >> because timer may be called from different thread than thread in
>>>> which
>>>> >> you
>>>> >> >> are inserting into queue.
>>>> >> >
>>>> >> > Right now it all happens in the same thread. When would it not be
>>>> in
>>>> >> > the same thread?
>>>> >> >
>>>> >> >> 2. AbstractType::setValue does delete mValue and set the new
>>>> pointer.
>>>> >> From
>>>> >> >> dbus signaler you are accessing mValue. I can reproduce crash in <
>>>> >> 50ms when
>>>> >> >> changing property every 1 ms.
>>>> >> >>
>>>> >> >
>>>> >> > Good catch. I'll look into it.
>>>> >>
>>>> >> What's the best way to reproduce it. I've got the following config
>>>> >> and I'm not able to reproduce it... It could be that my local
>>>> changes
>>>> >> are obfuscating reproduction though...
>>>> >>
>>>> >> {
>>>> >> "sources" : [
>>>> >> {
>>>> >> "name" : "ExampleSouce",
>>>> >> "path" :
>>>> >> "/usr/lib/automotive-message-broker/examplesourceplugin.so",
>>>> >> "delay" : "0"
>>>> >> }
>>>> >> ],
>>>> >> "sinks": [
>>>> >> {
>>>> >> "name" : "DBusSink",
>>>> >> "path" :
>>>> >> "/usr/lib/automotive-message-broker/dbussinkplugin.so",
>>>> >> "frequency" : "60"
>>>> >> }
>>>> >>
>>>> >> ]
>>>> >> }
>>>> >>
>>>> >>
>>>> >> >
>>>> >> >> I did some performance improvement, but I haven't been able to
>>>> compare
>>>> >> >> because of the crash of the original code :-)
>>>> >> >>
>>>> >> >> Marek
>>>> >> >>
>>>> >> >> On Jun 11, 2014 9:41 PM, "Marek Timko" <marek.timko1(a)gmail.com>
>>>> wrote:
>>>> >> >>>
>>>> >> >>> Just came to my mind: I have a very good experience with Intel
>>>> VTune
>>>> >> ;-)
>>>> >> >>> It was on Windows, but it's available also for Linux.
>>>> >> >>>
>>>> >> >>> https://software.intel.com/en-us/intel-vtune-amplifier-xe
>>>> >> >>>
>>>> >> >>> It's able to to show CPU usage per function.
>>>> >> >>>
>>>> >> >>> Marek
>>>> >> >>>
>>>> >> >>> On Tue, Jun 10, 2014 at 12:47 AM, marek.timko1(a)gmail.com
>>>> >> >>> <marek.timko1(a)gmail.com> wrote:
>>>> >> >>> >
>>>> >> >>> > Hi Kevron,
>>>> >> >>> > I see that there is still 8-9% CPU usage in performance
>>>> >> >>> > test(https://bugs.tizen.org/jira/browse/TIVI-2623).
>>>> >> >>>
>>>> >> >>> I haven't pushed some of the more impactful changes to tizen
>>>> yet. So
>>>> >> >>> the most recent test doesn't include those (0.11.809 vs 810).
>>>> >> >>>
>>>> >> >>> > I suggest to use valgrind with callgrind
>>>> plugin(--tool=callgrind).
>>>> >> >>> > QtCreator has a nice support plugin for valgrind.
>>>> >> >>> > You can find tho most often called methods(and time
>>>> consumption) and
>>>> >> >>> > improove the most critical then.
>>>> >> >>> >
>>>> >> >>>
>>>> >> >>> Agreed. I do use that tool in QtCreator. It's very nice.
>>>> There's a
>>>> >> >>> lot of data to sift through. So far I've only been addressing
>>>> the
>>>> >> >>> biggest pigs I can find, which are typically related to DBus
>>>> usage.
>>>> >> >>> But as that becomes less of an issue, I'll have to use the tools
>>>> to
>>>> >> >>> find smaller "pigs".
>>>> >> >>>
>>>> >> >>> > And I still think that you should replace all method calls with
>>>> >> >>> > arguments
>>>> >> >>> > passed by value with arguments passed by (const) reference. At
>>>> least
>>>> >> >>> > those
>>>> >> >>> > under lib folder - it affect all plugins and can be called very
>>>> >> often.
>>>> >> >>> > For example
>>>> >> AbstractRoutingEngine::setProperty(AsyncSetPropertyRequest
>>>> >> >>> > request) is called for every signal change, If I remember
>>>> correctly.
>>>> >> >>> > It also should reduce CPU usage. ( Imagine std::string ->
>>>> there is
>>>> >> >>> > always at
>>>> >> >>> > least one malloc, one memcopy and one free call for each
>>>> method call
>>>> >> >>> > with
>>>> >> >>> > arguments passed by value for each std::string argument).
>>>> >> >>> >
>>>> >> >>>
>>>> >> >>> Agreed. Passing by const ref makes sense.
>>>> >> >>>
>>>> >> >>> > Marek.
>>>> >> >>> >
>>>> >> >>> > Sent from my mobile
>>>> >> >>> >
>>>> >> >>> > _______________________________________________
>>>> >> >>> > AMB mailing list
>>>> >> >>> > AMB(a)lists.01.org
>>>> >> >>> > https://lists.01.org/mailman/listinfo/amb
>>>> >> >>> >
>>>> >>
>>>> >
>>>> > _______________________________________________
>>>> > AMB mailing list
>>>> > AMB(a)lists.01.org
>>>> > https://lists.01.org/mailman/listinfo/amb
>>>> >
>>>> >
>>>> -------------- next part --------------
>>>> An HTML attachment was scrubbed...
>>>> URL: <
>>>> http://lists.01.org/pipermail/amb/attachments/20140624/29eb02ea/attachmen...
>>>> >
>>>>
>>>> ------------------------------
>>>>
>>>> Subject: Digest Footer
>>>>
>>>> _______________________________________________
>>>> AMB mailing list
>>>> AMB(a)lists.01.org
>>>> https://lists.01.org/mailman/listinfo/amb
>>>>
>>>>
>>>> ------------------------------
>>>>
>>>> End of AMB Digest, Vol 3, Issue 5
>>>> *********************************
>>>>
>>>
>>> _______________________________________________
>>> AMB mailing list
>>> AMB(a)lists.01.org
>>> https://lists.01.org/mailman/listinfo/amb
>>>
>>>
>>
6 years, 5 months
- 1
- 0
New development procedure
by Rees, Kevron
All,
In order to accommodate and promote more community participation,
maintainers will no longer be permitted to directly push code changes
to the project. Any code changes must be submitted as pull requests.
This allows more transparency about the changes and allows for
community comments before changes are merged into the mainline branch.
I will leave pull requests open for comment for up to 5 days. If
there is community acceptance before that time we can merge it in
earlier. But if there are no comments or activity, the pull request
will be merged after 5 days.
Thanks,
Kevron
6 years, 5 months
- 1
- 0
Re: [AMB] AMB Digest, Vol 3, Issue 5
by Marek Timko
That is the problem. My plugins are calling routing engine from their own
threads.
How you can ensure that nobody will do it?
Routing engine e has to be thread safe.
How I can call routing engine from ambd main thread, If I have separate
thread for receiving CAN frames???
Marek
On Jun 25, 2014 2:35 AM, <amb-request(a)lists.01.org> wrote:
> Send AMB mailing list submissions to
> amb(a)lists.01.org
>
> To subscribe or unsubscribe via the World Wide Web, visit
> https://lists.01.org/mailman/listinfo/amb
> or, via email, send a message with subject or body 'help' to
> amb-request(a)lists.01.org
>
> You can reach the person managing the list at
> amb-owner(a)lists.01.org
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of AMB digest..."
>
>
> Today's Topics:
>
> 1. Re: AMB Performance (Rees, Kevron)
> 2. Re: AMB Performance (Marek Timko)
> 3. Re: AMB Performance (Kevron Rees)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Tue, 24 Jun 2014 13:22:15 -0700
> From: "Rees, Kevron" <kevron.m.rees(a)intel.com>
> To: Marek Timko <marek.timko1(a)gmail.com>
> Cc: amb(a)lists.01.org
> Subject: Re: [AMB] AMB Performance
> Message-ID:
> <
> CAFW5wYZEhm8D9jR7p3Ysk6UJReien9-4jrhRcyZc9hf0G-V2dA(a)mail.gmail.com>
> Content-Type: text/plain; charset=UTF-8
>
> On Tue, Jun 24, 2014 at 11:21 AM, Rees, Kevron <kevron.m.rees(a)intel.com>
> wrote:
> > On Tue, Jun 24, 2014 at 10:11 AM, Marek Timko <marek.timko1(a)gmail.com>
> wrote:
> >> Hi,
> >>
> >> I checked the latest code and I see 2 issues that can cause a crash.
> >> Both related to DBus signaler.
> >> 1. You have to protect queue with mutex,
> >> because timer may be called from different thread than thread in which
> you
> >> are inserting into queue.
> >
> > Right now it all happens in the same thread. When would it not be in
> > the same thread?
> >
> >> 2. AbstractType::setValue does delete mValue and set the new pointer.
> From
> >> dbus signaler you are accessing mValue. I can reproduce crash in < 50ms
> when
> >> changing property every 1 ms.
> >>
> >
> > Good catch. I'll look into it.
>
> What's the best way to reproduce it. I've got the following config
> and I'm not able to reproduce it... It could be that my local changes
> are obfuscating reproduction though...
>
> {
> "sources" : [
> {
> "name" : "ExampleSouce",
> "path" :
> "/usr/lib/automotive-message-broker/examplesourceplugin.so",
> "delay" : "0"
> }
> ],
> "sinks": [
> {
> "name" : "DBusSink",
> "path" :
> "/usr/lib/automotive-message-broker/dbussinkplugin.so",
> "frequency" : "60"
> }
>
> ]
> }
>
>
> >
> >> I did some performance improvement, but I haven't been able to compare
> >> because of the crash of the original code :-)
> >>
> >> Marek
> >>
> >> On Jun 11, 2014 9:41 PM, "Marek Timko" <marek.timko1(a)gmail.com> wrote:
> >>>
> >>> Just came to my mind: I have a very good experience with Intel VTune
> ;-)
> >>> It was on Windows, but it's available also for Linux.
> >>>
> >>> https://software.intel.com/en-us/intel-vtune-amplifier-xe
> >>>
> >>> It's able to to show CPU usage per function.
> >>>
> >>> Marek
> >>>
> >>> On Tue, Jun 10, 2014 at 12:47 AM, marek.timko1(a)gmail.com
> >>> <marek.timko1(a)gmail.com> wrote:
> >>> >
> >>> > Hi Kevron,
> >>> > I see that there is still 8-9% CPU usage in performance
> >>> > test(https://bugs.tizen.org/jira/browse/TIVI-2623).
> >>>
> >>> I haven't pushed some of the more impactful changes to tizen yet. So
> >>> the most recent test doesn't include those (0.11.809 vs 810).
> >>>
> >>> > I suggest to use valgrind with callgrind plugin(--tool=callgrind).
> >>> > QtCreator has a nice support plugin for valgrind.
> >>> > You can find tho most often called methods(and time consumption) and
> >>> > improove the most critical then.
> >>> >
> >>>
> >>> Agreed. I do use that tool in QtCreator. It's very nice. There's a
> >>> lot of data to sift through. So far I've only been addressing the
> >>> biggest pigs I can find, which are typically related to DBus usage.
> >>> But as that becomes less of an issue, I'll have to use the tools to
> >>> find smaller "pigs".
> >>>
> >>> > And I still think that you should replace all method calls with
> >>> > arguments
> >>> > passed by value with arguments passed by (const) reference. At least
> >>> > those
> >>> > under lib folder - it affect all plugins and can be called very
> often.
> >>> > For example
> AbstractRoutingEngine::setProperty(AsyncSetPropertyRequest
> >>> > request) is called for every signal change, If I remember correctly.
> >>> > It also should reduce CPU usage. ( Imagine std::string -> there is
> >>> > always at
> >>> > least one malloc, one memcopy and one free call for each method call
> >>> > with
> >>> > arguments passed by value for each std::string argument).
> >>> >
> >>>
> >>> Agreed. Passing by const ref makes sense.
> >>>
> >>> > Marek.
> >>> >
> >>> > Sent from my mobile
> >>> >
> >>> > _______________________________________________
> >>> > AMB mailing list
> >>> > AMB(a)lists.01.org
> >>> > https://lists.01.org/mailman/listinfo/amb
> >>> >
>
>
> ------------------------------
>
> Message: 2
> Date: Tue, 24 Jun 2014 22:49:49 +0200
> From: Marek Timko <marek.timko1(a)gmail.com>
> To: "Rees, Kevron" <kevron.m.rees(a)intel.com>
> Cc: amb(a)lists.01.org
> Subject: Re: [AMB] AMB Performance
> Message-ID:
> <CADNADdEHBG04qdi3jCR-Bpiu5mj6bvc7Y=2eoJzn=
> R3rLs0R6g(a)mail.gmail.com>
> Content-Type: text/plain; charset="utf-8"
>
> I used can plugin (source plugin, can be cansimplugin) and canplay utility(
> from can-utils) to send ~1000 can frames / changes of the VehicleSpeed per
> second.
> On Jun 24, 2014 10:22 PM, "Rees, Kevron" <kevron.m.rees(a)intel.com> wrote:
>
> > On Tue, Jun 24, 2014 at 11:21 AM, Rees, Kevron <kevron.m.rees(a)intel.com>
> > wrote:
> > > On Tue, Jun 24, 2014 at 10:11 AM, Marek Timko <marek.timko1(a)gmail.com>
> > wrote:
> > >> Hi,
> > >>
> > >> I checked the latest code and I see 2 issues that can cause a crash.
> > >> Both related to DBus signaler.
> > >> 1. You have to protect queue with mutex,
> > >> because timer may be called from different thread than thread in which
> > you
> > >> are inserting into queue.
> > >
> > > Right now it all happens in the same thread. When would it not be in
> > > the same thread?
> > >
> > >> 2. AbstractType::setValue does delete mValue and set the new pointer.
> > From
> > >> dbus signaler you are accessing mValue. I can reproduce crash in <
> 50ms
> > when
> > >> changing property every 1 ms.
> > >>
> > >
> > > Good catch. I'll look into it.
> >
> > What's the best way to reproduce it. I've got the following config
> > and I'm not able to reproduce it... It could be that my local changes
> > are obfuscating reproduction though...
> >
> > {
> > "sources" : [
> > {
> > "name" : "ExampleSouce",
> > "path" :
> > "/usr/lib/automotive-message-broker/examplesourceplugin.so",
> > "delay" : "0"
> > }
> > ],
> > "sinks": [
> > {
> > "name" : "DBusSink",
> > "path" :
> > "/usr/lib/automotive-message-broker/dbussinkplugin.so",
> > "frequency" : "60"
> > }
> >
> > ]
> > }
> >
> >
> > >
> > >> I did some performance improvement, but I haven't been able to compare
> > >> because of the crash of the original code :-)
> > >>
> > >> Marek
> > >>
> > >> On Jun 11, 2014 9:41 PM, "Marek Timko" <marek.timko1(a)gmail.com>
> wrote:
> > >>>
> > >>> Just came to my mind: I have a very good experience with Intel VTune
> > ;-)
> > >>> It was on Windows, but it's available also for Linux.
> > >>>
> > >>> https://software.intel.com/en-us/intel-vtune-amplifier-xe
> > >>>
> > >>> It's able to to show CPU usage per function.
> > >>>
> > >>> Marek
> > >>>
> > >>> On Tue, Jun 10, 2014 at 12:47 AM, marek.timko1(a)gmail.com
> > >>> <marek.timko1(a)gmail.com> wrote:
> > >>> >
> > >>> > Hi Kevron,
> > >>> > I see that there is still 8-9% CPU usage in performance
> > >>> > test(https://bugs.tizen.org/jira/browse/TIVI-2623).
> > >>>
> > >>> I haven't pushed some of the more impactful changes to tizen yet. So
> > >>> the most recent test doesn't include those (0.11.809 vs 810).
> > >>>
> > >>> > I suggest to use valgrind with callgrind plugin(--tool=callgrind).
> > >>> > QtCreator has a nice support plugin for valgrind.
> > >>> > You can find tho most often called methods(and time consumption)
> and
> > >>> > improove the most critical then.
> > >>> >
> > >>>
> > >>> Agreed. I do use that tool in QtCreator. It's very nice. There's a
> > >>> lot of data to sift through. So far I've only been addressing the
> > >>> biggest pigs I can find, which are typically related to DBus usage.
> > >>> But as that becomes less of an issue, I'll have to use the tools to
> > >>> find smaller "pigs".
> > >>>
> > >>> > And I still think that you should replace all method calls with
> > >>> > arguments
> > >>> > passed by value with arguments passed by (const) reference. At
> least
> > >>> > those
> > >>> > under lib folder - it affect all plugins and can be called very
> > often.
> > >>> > For example
> > AbstractRoutingEngine::setProperty(AsyncSetPropertyRequest
> > >>> > request) is called for every signal change, If I remember
> correctly.
> > >>> > It also should reduce CPU usage. ( Imagine std::string -> there is
> > >>> > always at
> > >>> > least one malloc, one memcopy and one free call for each method
> call
> > >>> > with
> > >>> > arguments passed by value for each std::string argument).
> > >>> >
> > >>>
> > >>> Agreed. Passing by const ref makes sense.
> > >>>
> > >>> > Marek.
> > >>> >
> > >>> > Sent from my mobile
> > >>> >
> > >>> > _______________________________________________
> > >>> > AMB mailing list
> > >>> > AMB(a)lists.01.org
> > >>> > https://lists.01.org/mailman/listinfo/amb
> > >>> >
> >
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: <
> http://lists.01.org/pipermail/amb/attachments/20140624/3255cdb2/attachmen...
> >
>
> ------------------------------
>
> Message: 3
> Date: Tue, 24 Jun 2014 17:35:03 -0700
> From: Kevron Rees <tripzero.kev(a)gmail.com>
> To: Marek Timko <marek.timko1(a)gmail.com>
> Cc: amb(a)lists.01.org
> Subject: Re: [AMB] AMB Performance
> Message-ID:
> <
> CAAOZRYVRBjp6Q6iof109GitxyjHXn2EtB+C-O-jcEBPpketu-g(a)mail.gmail.com>
> Content-Type: text/plain; charset="utf-8"
>
> Delay 0 produces about 300k changes in the example source. I'm not hitting
> a crash. Signaller uses the abstractproperty. So it shouldn't ever crash
> unless it was being changed by a different thread at the same time as the
> signal was being fired...
>
> Plugins shouldn't be calling any routing engine calls from different
> threads. RoutingEngine is not thread safe
> On Jun 24, 2014 1:50 PM, "Marek Timko" <marek.timko1(a)gmail.com> wrote:
>
> > I used can plugin (source plugin, can be cansimplugin) and canplay
> > utility( from can-utils) to send ~1000 can frames / changes of the
> > VehicleSpeed per second.
> > On Jun 24, 2014 10:22 PM, "Rees, Kevron" <kevron.m.rees(a)intel.com>
> wrote:
> >
> >> On Tue, Jun 24, 2014 at 11:21 AM, Rees, Kevron <kevron.m.rees(a)intel.com
> >
> >> wrote:
> >> > On Tue, Jun 24, 2014 at 10:11 AM, Marek Timko <marek.timko1(a)gmail.com
> >
> >> wrote:
> >> >> Hi,
> >> >>
> >> >> I checked the latest code and I see 2 issues that can cause a crash.
> >> >> Both related to DBus signaler.
> >> >> 1. You have to protect queue with mutex,
> >> >> because timer may be called from different thread than thread in
> which
> >> you
> >> >> are inserting into queue.
> >> >
> >> > Right now it all happens in the same thread. When would it not be in
> >> > the same thread?
> >> >
> >> >> 2. AbstractType::setValue does delete mValue and set the new pointer.
> >> From
> >> >> dbus signaler you are accessing mValue. I can reproduce crash in <
> >> 50ms when
> >> >> changing property every 1 ms.
> >> >>
> >> >
> >> > Good catch. I'll look into it.
> >>
> >> What's the best way to reproduce it. I've got the following config
> >> and I'm not able to reproduce it... It could be that my local changes
> >> are obfuscating reproduction though...
> >>
> >> {
> >> "sources" : [
> >> {
> >> "name" : "ExampleSouce",
> >> "path" :
> >> "/usr/lib/automotive-message-broker/examplesourceplugin.so",
> >> "delay" : "0"
> >> }
> >> ],
> >> "sinks": [
> >> {
> >> "name" : "DBusSink",
> >> "path" :
> >> "/usr/lib/automotive-message-broker/dbussinkplugin.so",
> >> "frequency" : "60"
> >> }
> >>
> >> ]
> >> }
> >>
> >>
> >> >
> >> >> I did some performance improvement, but I haven't been able to
> compare
> >> >> because of the crash of the original code :-)
> >> >>
> >> >> Marek
> >> >>
> >> >> On Jun 11, 2014 9:41 PM, "Marek Timko" <marek.timko1(a)gmail.com>
> wrote:
> >> >>>
> >> >>> Just came to my mind: I have a very good experience with Intel VTune
> >> ;-)
> >> >>> It was on Windows, but it's available also for Linux.
> >> >>>
> >> >>> https://software.intel.com/en-us/intel-vtune-amplifier-xe
> >> >>>
> >> >>> It's able to to show CPU usage per function.
> >> >>>
> >> >>> Marek
> >> >>>
> >> >>> On Tue, Jun 10, 2014 at 12:47 AM, marek.timko1(a)gmail.com
> >> >>> <marek.timko1(a)gmail.com> wrote:
> >> >>> >
> >> >>> > Hi Kevron,
> >> >>> > I see that there is still 8-9% CPU usage in performance
> >> >>> > test(https://bugs.tizen.org/jira/browse/TIVI-2623).
> >> >>>
> >> >>> I haven't pushed some of the more impactful changes to tizen yet.
> So
> >> >>> the most recent test doesn't include those (0.11.809 vs 810).
> >> >>>
> >> >>> > I suggest to use valgrind with callgrind plugin(--tool=callgrind).
> >> >>> > QtCreator has a nice support plugin for valgrind.
> >> >>> > You can find tho most often called methods(and time consumption)
> and
> >> >>> > improove the most critical then.
> >> >>> >
> >> >>>
> >> >>> Agreed. I do use that tool in QtCreator. It's very nice. There's
> a
> >> >>> lot of data to sift through. So far I've only been addressing the
> >> >>> biggest pigs I can find, which are typically related to DBus usage.
> >> >>> But as that becomes less of an issue, I'll have to use the tools to
> >> >>> find smaller "pigs".
> >> >>>
> >> >>> > And I still think that you should replace all method calls with
> >> >>> > arguments
> >> >>> > passed by value with arguments passed by (const) reference. At
> least
> >> >>> > those
> >> >>> > under lib folder - it affect all plugins and can be called very
> >> often.
> >> >>> > For example
> >> AbstractRoutingEngine::setProperty(AsyncSetPropertyRequest
> >> >>> > request) is called for every signal change, If I remember
> correctly.
> >> >>> > It also should reduce CPU usage. ( Imagine std::string -> there is
> >> >>> > always at
> >> >>> > least one malloc, one memcopy and one free call for each method
> call
> >> >>> > with
> >> >>> > arguments passed by value for each std::string argument).
> >> >>> >
> >> >>>
> >> >>> Agreed. Passing by const ref makes sense.
> >> >>>
> >> >>> > Marek.
> >> >>> >
> >> >>> > Sent from my mobile
> >> >>> >
> >> >>> > _______________________________________________
> >> >>> > AMB mailing list
> >> >>> > AMB(a)lists.01.org
> >> >>> > https://lists.01.org/mailman/listinfo/amb
> >> >>> >
> >>
> >
> > _______________________________________________
> > AMB mailing list
> > AMB(a)lists.01.org
> > https://lists.01.org/mailman/listinfo/amb
> >
> >
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: <
> http://lists.01.org/pipermail/amb/attachments/20140624/29eb02ea/attachmen...
> >
>
> ------------------------------
>
> Subject: Digest Footer
>
> _______________________________________________
> AMB mailing list
> AMB(a)lists.01.org
> https://lists.01.org/mailman/listinfo/amb
>
>
> ------------------------------
>
> End of AMB Digest, Vol 3, Issue 5
> *********************************
>
6 years, 5 months
- 1
- 0
AMB Performance
by marek.timko1＠gmail.com
Hi Kevron,
I see that there is still 8-9% CPU usage in performance test(https://bugs.tizen.org/jira/browse/TIVI-2623).
I suggest to use valgrind with callgrind plugin(--tool=callgrind).
QtCreator has a nice support plugin for valgrind.
You can find tho most often called methods(and time consumption) and improove the most critical then.
And I still think that you should replace all method calls with arguments passed by value with arguments passed by (const) reference. At least those under lib folder - it affect all plugins and can be called very often.
For example AbstractRoutingEngine::setProperty(AsyncSetPropertyRequest request) is called for every signal change, If I remember correctly.
It also should reduce CPU usage. ( Imagine std::string -> there is always at least one malloc, one memcopy and one free call for each method call with arguments passed by value for each std::string argument).
Marek.
Sent from my mobile
6 years, 5 months
- 4
- 7
- ← Newer
- 1
- Older →