Hi Christian,

Even though this is not needed by all developer, this is also required.

To satisfy this requirement, minimal code needs to be added.

I don’t think it take lots of code.

Then, what is your idea to support new requirement?

BR, Uze Choi

From: Christian Gran [mailto:g...@lynxtechnology.com] 
Sent: Wednesday, April 04, 2018 5:22 PM
To: 최우제 (Uze Choi); Maloor, Kishen; iotivity-dev@lists.iotivity.org
Subject: Re: [dev] API proposal for better userability in IoTivity Lite.

 

Hi, 

 

in my view a thread safe API is not needed by all developers.

Therefore it should not be part of the base stack - it should become an 
optional part as our main concern is code size and stack size.

 

thanks

  Christian

 





On 4. Apr 2018, at 10:16, 최우제 (Uze Choi) <uzc...@samsung.com> wrote:

 

Hi Kishen, Thank you feedback.

 

Let me clarify fundamental concern. 

I intend to “Remove the synchronization effort from the developer.”

Samsung have developed several product models since iotivity released.

Even though we guided to do synchronization from App, developer skipped it.

Considering the existing application using different protocol beyond 
iotivity(Lite), I expect usually has the separate thread or task for each 
response scenario.

When it comes to replacement of existing protocol stack, it is not easy to 
change the existing application structure. So they do not change the 
application model.

Critical fact is thread racing does not happens in normal case. Issue cannot 
happen in development stage or some test stage.

Only from intensive racing condition test, we may find it or we can find it in 
the real market. Even CTT cannot filter it.

We have experienced lots of cost to maintaining it.

This is the rational to make the API the thread safe.

I believe easy api is key factor for Lite version success considering various 
level of developers and situation.

 

>>  Regardless of how we can make upper layer working as multi thread concept, 
>> oc_rep is required to 

>> be made individually.

>> Without this implementation on beneath layer, how genius upper layer logic 
>> could be built, multi 

>> thread access is not possible.

 

> Why not? It would be thread-safe insofar as it would serialize access to the

> shared resource. The primary shared resource here is the (global) root-level 
> encoder

> that has a pointer to a buffer that is set internally.

 

>>  On top of current working concept, I just want to add additional API to 
>> work as local variable 

>>  concept for oc_rep.

 

> I understand that you’d prefer to avoid using a global encoder.

 

>>  This API is independent from thread locking. 

 

> Okay, I thought your primary goal for change here was thread-safety. 

> But as I clarified, even making oc_rep_* fully thread-safe does not obviate

> the need for locking generally around other APIs.

> Keep in mind however, that all encoder calls that occur inside resource

> handlers don’t have to bother about thread-safety as they always run in that 
> single

> thread. Only when you’re encoding a PUT/POST request payload from a 

> separate application thread is it a problem, which is solved by locking.

 

I believe the separation application code for PUT/POST/NOTIFY is a common 
requirement.

And already I share my idea upper.

 

>>  Any my API change proposal give the consistence for oc_do_get and 
>> oc_do_post.

>>  ..

>> Just separation of concept, to separate the oc_rep making and do_post logic.

 

> Understood - sounds like you’d want to have a single oc_do_post(),

> much like we have oc_do_get(), instead of oc_init_post() <payload encode> 
> oc_do_post()?

> Of course, oc_init_post() initializes that root-level encoder and sets the 
> buffer.

> (on the server-side, this happens prior to invoking the resource handler)

> There are other steps that happen automatically inside. So, you’d have to 
> assess

> the entire impact and consequences of any refactoring.

 

handler buffer Initialization and encoder are independent.

There is no need to buffer Initialization prior to filling in CborEncoder.

This is additional API so there is no architecture change or causing impact.

 

>> This will give more flexibility if developer need and want. Otherwise, no 
>> change without this 

>> API usage.

 

> Its not a bad idea. But it just isn’t clear to me (yet) what existential

> problem this refactoring solves, because it seemingly replaces (or adds) one 
> set of

> steps with (to) another set. 

> Often times, that answer becomes easier to spot while looking at working code.

 

Let;s assume three resources are there.

>From each code block creates the representation, and send response.

Sometime, these representations need to be combined with collection association.

Representation is to be created and build up. Send response to be done.

For this scenario, separation of do_post_init and representation build need to 
be separated.

representation build need to be done separate thread.

 

BR, Uze Choi

From: Maloor, Kishen [ <mailto:kishen.mal...@intel.com> 
mailto:kishen.mal...@intel.com] 
Sent: Wednesday, April 04, 2018 8:16 AM
To: 최우제 (Uze Choi);  <mailto:iotivity-dev@lists.iotivity.org> 
iotivity-dev@lists.iotivity.org
Subject: Re: [dev] API proposal for better userability in IoTivity Lite.

 

Hi Uze,

 

> Regardless of how we can make upper layer working as multi thread concept, 
> oc_rep is required to 

> be made individually.

> Without this implementation on beneath layer, how genius upper layer logic 
> could be built, multi 

> thread access is not possible.

 

Why not? It would be thread-safe insofar as it would serialize access to the

shared resource. The primary shared resource here is the (global) root-level 
encoder

that has a pointer to a buffer that is set internally.

 

> On top of current working concept, I just want to add additional API to work 
> as local variable 

> concept for oc_rep.

 

I understand that you’d prefer to avoid using a global encoder.

 

> This API is independent from thread locking. 

 

Okay, I thought your primary goal for change here was thread-safety. 

But as I clarified, even making oc_rep_* fully thread-safe does not obviate

the need for locking generally around other APIs.

Keep in mind however, that all encoder calls that occur inside resource

handlers don’t have to bother about thread-safety as they always run in that 
single

thread. Only when you’re encoding a PUT/POST request payload from a 

separate application thread is it a problem, which is solved by locking.

 

> Any my API change proposal give the consistence for oc_do_get and oc_do_post.

> ..

> Just separation of concept, to separate the oc_rep making and do_post logic.

 

Understood - sounds like you’d want to have a single oc_do_post(),

much like we have oc_do_get(), instead of oc_init_post() <payload encode> 
oc_do_post()?

Of course, oc_init_post() initializes that root-level encoder and sets the 
buffer.

(on the server-side, this happens prior to invoking the resource handler)

There are other steps that happen automatically inside. So, you’d have to assess

the entire impact and consequences of any refactoring.

 

> This will give more flexibility if developer need and want. Otherwise, no 
> change without this 

> API usage.

 

Its not a bad idea. But it just isn’t clear to me (yet) what existential

problem this refactoring solves, because it seemingly replaces (or adds) one 
set of

steps with (to) another set. 

Often times, that answer becomes easier to spot while looking at working code.

 

Thanks,

-Kishen.

 

-

Kishen Maloor

Intel Open Source Technology Center

 

From: "최우제 (Uze Choi)" < <mailto:uzc...@samsung.com> uzc...@samsung.com>
Date: Monday, April 2, 2018 at 10:17 PM
To: Kishen Maloor < <mailto:kishen.mal...@intel.com> kishen.mal...@intel.com>, 
" <mailto:iotivity-dev@lists.iotivity.org> iotivity-dev@lists.iotivity.org" < 
<mailto:iotivity-dev@lists.iotivity.org> iotivity-dev@lists.iotivity.org>
Subject: RE: [dev] API proposal for better userability in IoTivity Lite.

 

Hi Kishen,

 

I appreciate your comment and effort in advance.

 

However, my proposal is not to change the current concept. Please understand 
this.

On top of current working concept, I just want to add additional API to work as 
local variable concept for oc_rep.

This will give more flexibility if developer need and want. Otherwise, no 
change without this API usage.

 

Regardless of how we can make upper layer working as multi thread concept, 
oc_rep is required to be made individually.

Without this implementation on beneath layer, how genius upper layer logic 
could be built, multi thread access is not possible.

 

Any my API change proposal give the consistence for oc_do_get and oc_do_post.

This API is independent from thread locking. Just separation of concept, to 
separate the oc_rep making and do_post logic.

Furthermore, this API add do not harm existing API concept also.

 

Furthermore, current oc_rep_start_root_object() API is error prone with the 
responsibility for oc_rep_end_object().

However, if oc_rep_create…() API create the end ‘]’ brace. And we can add the 
key & value set in the middle.

this will help for better understanding. This is similar to exsiting IoTivity 
API concept, I think.

We can make it enabled with patch into tiny_cbor.

 

BR, Uze Choi

From: Maloor, Kishen [ <mailto:kishen.mal...@intel.com> 
mailto:kishen.mal...@intel.com] 
Sent: Tuesday, April 03, 2018 12:47 PM
To: 최우제 (Uze Choi);  <mailto:iotivity-dev@lists.iotivity.org> 
iotivity-dev@lists.iotivity.org
Subject: Re: [dev] API proposal for better userability in IoTivity Lite.

 

Hi Uze,

 

As the framework is single-threaded by design, many internal

modules operate on global structures. So, the global CborEncoder

objects used in oc_rep aren’t an exception, and are in fact an

efficiency that takes advantage of the single-threaded design.

Consequently, just modifying oc_rep to allocate them dynamically

wouldn’t make the APIs thread-safe.

 

However, (equivalent) thread-safe APIs (with the same signature)

can be implemented (wherever they’re necessary) in a thin upper

layer, as an accessory, to be used by multi-threaded applications that

don’t want to bother with locking themselves. I imagine this’ll be particularly

useful in clients and to a limited extent in servers.

I am experimenting with such a scheme and should post it once its done.

 

Thanks,

-Kishen.

 

-

Kishen Maloor

Intel Open Source Technology Center

 

From: < <mailto:iotivity-dev-boun...@lists.iotivity.org> 
iotivity-dev-boun...@lists.iotivity.org> on behalf of "최우제 (Uze Choi)" < 
<mailto:uzc...@samsung.com> uzc...@samsung.com>
Date: Monday, April 2, 2018 at 1:58 AM
To: " <mailto:iotivity-dev@lists.iotivity.org> iotivity-dev@lists.iotivity.org" 
< <mailto:iotivity-dev@lists.iotivity.org> iotivity-dev@lists.iotivity.org>
Subject: [dev] API proposal for better userability in IoTivity Lite.

 

Hi Kishen,

 

Let me propose new APIs to satisfy followings.

 

Two requirements

: Multi Thread Application Code requirement

: Easy-to use with consistent API signature

 

Problem

: Singleton CborEncoder Root object prevent creating payload in multithread 
race condition.

   - Get Response, Notify in Server side and Client side cannot be handled from 
different thread.
           It could be done by synchronization handling from App side but not 
easy to use.

 

How to….

 

Server Side AS-IS

oc_rep_start_root_object();

oc_process_baseline_interface(request->resource);

oc_rep_set_boolean(root, state, state);

oc_rep_end_root_object();

oc_send_response(request, OC_STATUS_OK);

 

Server Side TO-BE

CborEncoder new_rep = oc_rep_create_root_object();  // New Additional : make 
opaque type for CborEncoder to hide it

oc_process_baseline_interface(new_rep, request->resource); // New Additional 
(with parameter adding)

oc_rep_set_boolean(new_rep, state, state);

oc_rep_end_root_object(new_rep);          // we can remove this call with 
tinycbor update.

oc_send_response(request, new_rep, OC_STATUS_OK);        // New Additional 
(with parameter adding), refer new_rep with g_encoder.

 

Client Side AS-IS

oc_init_post(a_light, light_server, NULL, &post2_light, LOW_QOS, NULL)

oc_rep_start_root_object();

oc_rep_set_boolean(root, state, state);

oc_rep_end_root_object();

oc_do_post();

 

Client Side TO-BE

// oc_init_post(a_light, light_server, NULL, &post2_light, LOW_QOS, NULL) ß 
This can be skipped because params are set in new oc_do_post.

CborEncoder new_rep = oc_rep_create_root_object();

oc_rep_set_boolean(new_rep, state, state);

oc_rep_end_root_object(new_rep);          // we can remove this call with 
tinycbor update.

oc_do_post(a_light, light_server, NULL, new_rep, &post2_light, LOW_QOS, NULL); 
// New Additional (with parameter adding), refer new_rep with g_encoder, This 
will make API consistent with oc_do_get(……)

 

--- Sumary New API List. ---

CborEncoder_opaque oc_rep_create_root_object()

oc_process_baseline_interface(new_rep, request->resource) // adding param

oc_send_response(request, new_rep, OC_STATUS_OK); // adding param

oc_do_post(…..) // adding param

_______________________________________________
iotivity-dev mailing list
 <mailto:iotivity-dev@lists.iotivity.org> iotivity-dev@lists.iotivity.org
 <https://lists.iotivity.org/mailman/listinfo/iotivity-dev> 
https://lists.iotivity.org/mailman/listinfo/iotivity-dev

 

_______________________________________________
iotivity-dev mailing list
iotivity-dev@lists.iotivity.org
https://lists.iotivity.org/mailman/listinfo/iotivity-dev

Reply via email to