How to get a datasource to access a dataobject<T>

I was working on the keeps_last_written / data_sample implementation in
OutputPort and realized that the way it is implemented right now is not
thread-safe ... and should be

So, I wanted to replace the ValueDataSource<T> by a DataObject<T>.
Except I don't know how to get a data source for DataObject<T> (the
datasource is used by the task browser I guess ...)

Hints ?

Sylvain

How to get a datasource to access a dataobject<T>

On Thu, Feb 3, 2011 at 2:17 PM, Sylvain Joyeux <sylvain [dot] joyeux [..] ...> wrote:
> I was working on the keeps_last_written / data_sample implementation in
> OutputPort and realized that the way it is implemented right now is not
> thread-safe ... and should be

Only keeps or also data_sample ? Is it the creation of a connection
that races with setDataSample() and write() ? Should setDataSample()
be thread-safe ?

>
> So, I wanted to replace the ValueDataSource<T> by a DataObject<T>.
> Except I don't know how to get a data source for DataObject<T> (the
> datasource is used by the task browser I guess ...)
>
> Hints ?

The DataSource API is inherently thread-unsafe because it returns a
(const) reference to its contents, which might be overwritten at any
time by a set(). So there's no possibility to make a Datasource
thread-safe. DataObject is a different API which is thread-safe, and
can't inherit from DataSource.

So your question is if we can eliminate getDataSource() from the
*output* ports API and internally use a DataObject, OR write a
DataSource wrapper around DataObject..

I think we can adapt the taskbrowser to read the last written sample
in another way (but that would involve using the 'last()' operation),
but a DataSource that does a Get() on a DataObject and caches the
result might be pretty straightforward too.

Peter

How to get a datasource to access a dataobject<T>

On 02/03/2011 03:19 PM, Peter Soetens wrote:
> On Thu, Feb 3, 2011 at 2:17 PM, Sylvain Joyeux<sylvain [dot] joyeux [..] ...> wrote:
>> I was working on the keeps_last_written / data_sample implementation in
>> OutputPort and realized that the way it is implemented right now is not
>> thread-safe ... and should be
>
> Only keeps or also data_sample ? Is it the creation of a connection
> that races with setDataSample() and write() ? Should setDataSample()
> be thread-safe ?
Both of them. connectionAdded is reading both and can be called from any
thread (it is part of the connection mechanism) while we can assume that
setDataSample and write() are called from the task thread.

>
>>
>> So, I wanted to replace the ValueDataSource<T> by a DataObject<T>.
>> Except I don't know how to get a data source for DataObject<T> (the
>> datasource is used by the task browser I guess ...)
>>
>> Hints ?
>
> The DataSource API is inherently thread-unsafe because it returns a
> (const) reference to its contents, which might be overwritten at any
> time by a set().
??? From my memory, the plain DataSource::get() returns a value.
rvalue() returns a const reference, but might not be available
(depending on the datasource). Or I missed something.

> So there's no possibility to make a Datasource
> thread-safe. DataObject is a different API which is thread-safe, and
> can't inherit from DataSource.
Obviously not.

> So your question is if we can eliminate getDataSource() from the
> *output* ports API and internally use a DataObject, OR write a
> DataSource wrapper around DataObject..
>
> I think we can adapt the taskbrowser to read the last written sample
> in another way (but that would involve using the 'last()' operation),
> but a DataSource that does a Get() on a DataObject and caches the
> result might be pretty straightforward too.
Well. I would personally prefer dropping getDataSource and use the
last() operation in the task browser. In any case, we would need to
synchronize on this as I won't touch the task browser.
--
Sylvain Joyeux (Dr.Ing.)
Space & Security Robotics

!!! Achtung, neue Telefonnummer!!!

Standort Bremen:
DFKI GmbH
Robotics Innovation Center
Robert-Hooke-Straße 5
28359 Bremen, Germany

Phone: +49 (0)421 178-454136
Fax: +49 (0)421 218-454150
E-Mail: robotik [..] ...

Weitere Informationen: http://www.dfki.de/robotik
-----------------------------------------------------------------------
Deutsches Forschungszentrum fuer Kuenstliche Intelligenz GmbH
Firmensitz: Trippstadter Straße 122, D-67663 Kaiserslautern
Geschaeftsfuehrung: Prof. Dr. Dr. h.c. mult. Wolfgang Wahlster
(Vorsitzender) Dr. Walter Olthoff
Vorsitzender des Aufsichtsrats: Prof. Dr. h.c. Hans A. Aukes
Amtsgericht Kaiserslautern, HRB 2313
Sitz der Gesellschaft: Kaiserslautern (HRB 2313)
USt-Id.Nr.: DE 148646973
Steuernummer: 19/673/0060/3
-----------------------------------------------------------------------
--
Orocos-Dev mailing list
Orocos-Dev [..] ...
http://lists.mech.kuleuven.be/mailman/listinfo/orocos-dev

How to get a datasource to access a dataobject<T>

On Thu, Feb 3, 2011 at 3:29 PM, Sylvain Joyeux <sylvain [dot] joyeux [..] ...> wrote:
> On 02/03/2011 03:19 PM, Peter Soetens wrote:
>>
>> On Thu, Feb 3, 2011 at 2:17 PM, Sylvain Joyeux<sylvain [dot] joyeux [..] ...>
>>  wrote:
>>>
>>> I was working on the keeps_last_written / data_sample implementation in
>>> OutputPort and realized that the way it is implemented right now is not
>>> thread-safe ... and should be
>>
>> Only keeps or also data_sample ? Is it the creation of a connection
>> that races with setDataSample() and write() ? Should setDataSample()
>> be thread-safe ?
>
> Both of them. connectionAdded is reading both and can be called from any
> thread (it is part of the connection mechanism) while we can assume that
> setDataSample and write() are called from the task thread.

Yes. But I was wondering if setDataSample() is useful in the task's
thread. We could document it as not thread-safe or do you see a use
case where it clashes a lot ? We initially only use setDataSample in
configureHook(), which is during deployment a moment where connections
are not created or removed. We typically don't call it from
updateHook().

>
>>
>>>
>>> So, I wanted to replace the ValueDataSource<T>  by a DataObject<T>.
>>> Except I don't know how to get a data source for DataObject<T>  (the
>>> datasource is used by the task browser I guess ...)
>>>
>>> Hints ?
>>
>> The DataSource API is inherently thread-unsafe because it returns a
>> (const) reference to its contents, which might be overwritten at any
>> time by a set().
>
> ??? From my memory, the plain DataSource::get() returns a value. rvalue()
> returns a const reference, but might not be available (depending on the
> datasource). Or I missed something.

In 2.0, rvalue() always returns a valid address in memory, in 1.x it
was allowed to return a reference to null. The latter was a time-bomb
imho.

>
>> So there's no possibility to make a Datasource
>> thread-safe. DataObject is a different API which is thread-safe, and
>> can't inherit from DataSource.
>
> Obviously not.
>
>> So your question is if we can eliminate getDataSource() from the
>> *output* ports API and internally use a DataObject, OR write a
>> DataSource wrapper around DataObject..
>>
>> I think we can adapt the taskbrowser to read the last written sample
>> in another way (but that would involve using the 'last()' operation),
>> but a DataSource that does a Get() on a DataObject and caches the
>> result might be pretty straightforward too.
>
> Well. I would personally prefer dropping getDataSource and use the last()
> operation in the task browser. In any case, we would need to synchronize on
> this as I won't touch the task browser.

I knew you would :-) I propose that you propose a patch according to
your best insights and then we can see what effects it has on the rest
and how it can be merged.

Peter
--
Orocos-Dev mailing list
Orocos-Dev [..] ...
http://lists.mech.kuleuven.be/mailman/listinfo/orocos-dev

How to get a datasource to access a dataobject<T>

On 02/03/2011 03:53 PM, Peter Soetens wrote:
> On Thu, Feb 3, 2011 at 3:29 PM, Sylvain Joyeux<sylvain [dot] joyeux [..] ...> wrote:
>> On 02/03/2011 03:19 PM, Peter Soetens wrote:
>>>
>>> On Thu, Feb 3, 2011 at 2:17 PM, Sylvain Joyeux<sylvain [dot] joyeux [..] ...>
>>> wrote:
>>>>
>>>> I was working on the keeps_last_written / data_sample implementation in
>>>> OutputPort and realized that the way it is implemented right now is not
>>>> thread-safe ... and should be
>>>
>>> Only keeps or also data_sample ? Is it the creation of a connection
>>> that races with setDataSample() and write() ? Should setDataSample()
>>> be thread-safe ?
>>
>> Both of them. connectionAdded is reading both and can be called from any
>> thread (it is part of the connection mechanism) while we can assume that
>> setDataSample and write() are called from the task thread.
>
> Yes. But I was wondering if setDataSample() is useful in the task's
> thread. We could document it as not thread-safe or do you see a use
> case where it clashes a lot ? We initially only use setDataSample in
> configureHook(), which is during deployment a moment where connections
> are not created or removed. We typically don't call it from
> updateHook().
For me, setDataSample is definitely part of the API that should be
available from the task. I guess we could add a constraint that it must
be called from the constructor, at the price of memory consumption (one
would have to initialize with bigger buffers than at configuration time)

I don't like the "configureHook() and connections should not clash"
constraint though. It is the case in Roby right now, but that's
something I'd like to lift at some point as configuration does take
quite a bit of time for some modules, so parallelizing would not hurt.

In any case, there is still a problem between write() and
connectionAdded(). And write() *does* belong in the task's thread ;-)

>>>> So, I wanted to replace the ValueDataSource<T> by a DataObject<T>.
>>>> Except I don't know how to get a data source for DataObject<T> (the
>>>> datasource is used by the task browser I guess ...)
>>>>
>>>> Hints ?
>>>
>>> The DataSource API is inherently thread-unsafe because it returns a
>>> (const) reference to its contents, which might be overwritten at any
>>> time by a set().
>>
>> ??? From my memory, the plain DataSource::get() returns a value. rvalue()
>> returns a const reference, but might not be available (depending on the
>> datasource). Or I missed something.
>
> In 2.0, rvalue() always returns a valid address in memory, in 1.x it
> was allowed to return a reference to null. The latter was a time-bomb
> imho.
I would agree. Now, get() does return a copy, no ?

>> Well. I would personally prefer dropping getDataSource and use the last()
>> operation in the task browser. In any case, we would need to synchronize on
>> this as I won't touch the task browser.
>
> I knew you would :-) I propose that you propose a patch according to
> your best insights and then we can see what effects it has on the rest
> and how it can be merged.
OK. We'll need to "talk" anyway about what's pending in rock-rtt
--
Sylvain Joyeux (Dr.Ing.)
Space & Security Robotics

!!! Achtung, neue Telefonnummer!!!

Standort Bremen:
DFKI GmbH
Robotics Innovation Center
Robert-Hooke-Straße 5
28359 Bremen, Germany

Phone: +49 (0)421 178-454136
Fax: +49 (0)421 218-454150
E-Mail: robotik [..] ...

Weitere Informationen: http://www.dfki.de/robotik
-----------------------------------------------------------------------
Deutsches Forschungszentrum fuer Kuenstliche Intelligenz GmbH
Firmensitz: Trippstadter Straße 122, D-67663 Kaiserslautern
Geschaeftsfuehrung: Prof. Dr. Dr. h.c. mult. Wolfgang Wahlster
(Vorsitzender) Dr. Walter Olthoff
Vorsitzender des Aufsichtsrats: Prof. Dr. h.c. Hans A. Aukes
Amtsgericht Kaiserslautern, HRB 2313
Sitz der Gesellschaft: Kaiserslautern (HRB 2313)
USt-Id.Nr.: DE 148646973
Steuernummer: 19/673/0060/3
-----------------------------------------------------------------------
--
Orocos-Dev mailing list
Orocos-Dev [..] ...
http://lists.mech.kuleuven.be/mailman/listinfo/orocos-dev

How to get a datasource to access a dataobject<T>

On Thu, Feb 3, 2011 at 4:06 PM, Sylvain Joyeux <sylvain [dot] joyeux [..] ...> wrote:
> On 02/03/2011 03:53 PM, Peter Soetens wrote:
>>
>> On Thu, Feb 3, 2011 at 3:29 PM, Sylvain Joyeux<sylvain [dot] joyeux [..] ...>
>>  wrote:
>>>
>>> On 02/03/2011 03:19 PM, Peter Soetens wrote:
>>>>
>>>> On Thu, Feb 3, 2011 at 2:17 PM, Sylvain Joyeux<sylvain [dot] joyeux [..] ...>
>>>>  wrote:
>>>>>
>>>>> I was working on the keeps_last_written / data_sample implementation in
>>>>> OutputPort and realized that the way it is implemented right now is not
>>>>> thread-safe ... and should be
>>>>
>>>> Only keeps or also data_sample ? Is it the creation of a connection
>>>> that races with setDataSample() and write() ? Should setDataSample()
>>>> be thread-safe ?
>>>
>>> Both of them. connectionAdded is reading both and can be called from any
>>> thread (it is part of the connection mechanism) while we can assume that
>>> setDataSample and write() are called from the task thread.
>>
>> Yes. But I was wondering if setDataSample() is useful in the task's
>> thread. We could document it as not thread-safe or do you see a use
>> case where it clashes a lot ? We initially only use setDataSample in
>> configureHook(), which is during deployment a moment where connections
>> are not created or removed. We typically don't call it from
>> updateHook().
>
> For me, setDataSample is definitely part of the API that should be available
> from the task. I guess we could add a constraint that it must be called from
> the constructor, at the price of memory consumption (one would have to
> initialize with bigger buffers than at configuration time)
>
> I don't like the "configureHook() and connections should not clash"
> constraint though. It is the case in Roby right now, but that's something
> I'd like to lift at some point as configuration does take quite a bit of
> time for some modules, so parallelizing would not hurt.

I agree 100% that it would be better not to have this constraint. It's
just the thought of the footprint each port will have that is scaring
me. Maybe I'm worrying about the wrong thing, but DataObject ain't
gentle on the memory.

>
> In any case, there is still a problem between write() and connectionAdded().
> And write() *does* belong in the task's thread ;-)

This one needs to be fixed properly for sure.

>
>>>>> So, I wanted to replace the ValueDataSource<T>    by a DataObject<T>.
>>>>> Except I don't know how to get a data source for DataObject<T>    (the
>>>>> datasource is used by the task browser I guess ...)
>>>>>
>>>>> Hints ?
>>>>
>>>> The DataSource API is inherently thread-unsafe because it returns a
>>>> (const) reference to its contents, which might be overwritten at any
>>>> time by a set().
>>>
>>> ??? From my memory, the plain DataSource::get() returns a value. rvalue()
>>> returns a const reference, but might not be available (depending on the
>>> datasource). Or I missed something.
>>
>> In 2.0, rvalue() always returns a valid address in memory, in 1.x it
>> was allowed to return a reference to null. The latter was a time-bomb
>> imho.
>
> I would agree. Now, get() does return a copy, no ?

Yes.

>
>>> Well. I would personally prefer dropping getDataSource and use the last()
>>> operation in the task browser. In any case, we would need to synchronize
>>> on
>>> this as I won't touch the task browser.
>>
>> I knew you would :-) I propose that you propose a patch according to
>> your best insights and then we can see what effects it has on the rest
>> and how it can be merged.
>
> OK. We'll need to "talk" anyway about what's pending in rock-rtt

You bet !

Peter
--
Orocos-Dev mailing list
Orocos-Dev [..] ...
http://lists.mech.kuleuven.be/mailman/listinfo/orocos-dev