The callback function parameter in multithread

[Darwin Lalo]

I have a lot of code like this:

VOID CALLBACK TimerRoutine(PVOID lpParam)
{
long nTaskid = (long)lpParam;

GObj *obj;
if( mapThreadSafe.find( nTaskid, obj )) // mapThreadSafe is a
hash_map, thread safe
{
obj->invoke();
}
}

someone think it's inefficient and code like this:

VOID CALLBACK TimerRoutine(PVOID lpParam)
{
GObj *obj = (GObj *)lpParam;
obj->invoke();
obj->Release();
}

I was confused.I think a hash_map is good enough and payable.
it can avoid a lot of problem in a complex environment.
what is your position?

 

[Alf P. Steinbach]

* Darwin Lalo:

I have a lot of code like this:

VOID CALLBACK TimerRoutine(PVOID lpParam)
{
long nTaskid = (long)lpParam;

GObj *obj;
if( mapThreadSafe.find( nTaskid, obj )) // mapThreadSafe is a
hash_map, thread safe
{
obj->invoke();
}
}

someone think it's inefficient and code like this:

VOID CALLBACK TimerRoutine(PVOID lpParam)
{
GObj *obj = (GObj *)lpParam;
obj->invoke();
obj->Release();
}

I was confused.I think a hash_map is good enough and payable.
it can avoid a lot of problem in a complex environment.
what is your position?

I think it's a good idea to get rid of the void pointers (isolate any
essential usage in some common small thing down in the depths), the C
style casts (unsafe for maintenance), the misleading and generally
unreadable Hungarian prefix notation (which has no advantage today), the
unnecessary macros like VOID for void (and macros in general), and the
manual redundant resource management like obj->Release (replace with
smart pointers, above you have both a deallocation responsibility
problem and an exception unsafety problem, both solved by better way).

That will improve the code a lot, I think.

 

[Darwin Lalo]

obj->Release is bad. ref_count_ptr is good. I agree.
but sometime System API need a PVOID parameter like

BOOL CreateTimer( TIMERCALLBACK Callback, PVOID Parameter, DWORD
DueTime)

The Parameter above can't be ref_count_ptr. how do I?

like this?

? struct TimerParameter{
ref_count_ptr<GObj> obj;
};

CALL {
TimerParameter *p = new TimerParameter;
CreateTimer(TimerRoutine, p, 10);
}

VOID CALLBACK TimerRoutine(PVOID lpParam)
{
TimerParameter *p = (TimerParameter *)lpParam;
ref_count_ptr<GObj> obj = p->obj;
delete p;

obj->invoke();
}

thank you,maybe I am newbie.

 

[Ian Collins]

obj->Release is bad. ref_count_ptr is good. I agree.
but sometime System API need a PVOID parameter like

You should quote context in your reply, not everyone uses google!

BOOL CreateTimer( TIMERCALLBACK Callback, PVOID Parameter, DWORD
DueTime)

The Parameter above can't be ref_count_ptr. how do I?

You can't in this case, you're stuck with what the API requires. Even
though the API uses ugly macros for parameter types, you code will
probably be easier to read if you don't.

Back to your original question, follow Alf's advice on variable names.

Considering the above CreateTimer binds an object (Parameter), to a
function (Callback), why bother with the map?

VOID CALLBACK TimerRoutine(PVOID lpParam)
{
TimerParameter *p = (TimerParameter *)lpParam;

Don't use C casts, in this case, use static_cast.

 

[Darwin Lalo]

You should quote context in your reply, not everyone uses google!

You can't in this case, you're stuck with what the API requires. Even
though the API uses ugly macros for parameter types, you code will
probably be easier to read if you don't.

Back to your original question, follow Alf's advice on variable names.

Considering the above CreateTimer binds an object (Parameter), to a
function (Callback), why bother with the map?

I don't know " binds an object (Parameter), to a function (Callback) "

this is the only sentence about my question.

you mean "functor" ?

 

[Ian Collins]

I don't know " binds an object (Parameter), to a function (Callback) "
this is the only sentence about my question.
you mean "functor" ?

I was assuming that "TIMERCALLBACK" is a function pointer, but looking
at the function definition, maybe it isn't. What does "VOID CALLBACK"
mean? Is it a compiler extension?

maybe reinterpret_cast

No, static_cast in this case (assuming PVOID expands to void*).

 

[Darwin Lalo]

I was assuming that "TIMERCALLBACK" is a function pointer,

yes

typedef void (* TIMERCALLBACK) (void *);

but looking
at the function definition, maybe it isn't. What does "VOID CALLBACK"
mean? Is it a compiler extension?

"VOID CALLBACK" is just " void " ,ok?

 

[Ian Collins]

yes

typedef void (* TIMERCALLBACK) (void *);




"VOID CALLBACK" is just " void " ,ok?

OK,, so back to your original question, when you call CreateTimer, you
pass in a function pointer and a pointer to an object that will be
passed to the function when the timer expires.

So the logic you had in your original function was superfluous.
provided you pass in a unique object, it will be passed to you callback.

 

[Maxim Yegorushkin]

I have a lot of code like this:

VOID CALLBACK TimerRoutine(PVOID lpParam)
{
long nTaskid = (long)lpParam;

GObj *obj;
if( mapThreadSafe.find( nTaskid, obj )) // mapThreadSafe is a
hash_map, thread safe
{
obj->invoke();
}
}

someone think it's inefficient and code like this:

It is indeed.

VOID CALLBACK TimerRoutine(PVOID lpParam)
{
GObj *obj = (GObj *)lpParam;
obj->invoke();
obj->Release();
}

The above is the proper way of writing efficient code using C-style
callbacks.

I was confused.I think a hash_map is good enough and payable.
it can avoid a lot of problem in a complex environment.
what is your position?

First of all, a thread safe container is pretty much always a bad idea
because it does not work if you want to do more than one call in an
atomic sequence. This is why external locking is preferred.

Second, you don't really need a map here. Make it simple...

 

[Darwin Lalo]

thank you for your straight answer

It is indeed.


The above is the proper way of writing efficient code using C-style
callbacks.


First of all, a thread safe container is pretty much always a bad idea
because it does not work if you want to do more than one call in an
atomic sequence. This is why external locking is preferred.

Second, you don't really need a map here. Make it simple...

my map is like this:

template<class Key,class Type>
class simple_map{
axis::CriticalSection<true> _cs;
public:
bool find(const Key& _key,Type& _val){
axis::CriticalSectionLock lock(_cs);
std::map<Key,Type>::const_iterator _ite = _map.find(_key);
if( _ite == _map.end() )return false;
_val = _ite->second;
return true;
}
bool find(const Key& _key){
axis::CriticalSectionLock lock(_cs);
return ( _map.find(_key) != _map.end() );
}
template<class T,class P>
bool apply(T *_buddy, bool (T::*_fun)(Key key, Type type, P
parameter), P& _parameter){
axis::CriticalSectionLock lock(_cs);
_ite = _map.begin();
while( _ite != _map.end()){
Type& _tval = _ite->second;
const Key& _key = _ite->first;
++_ite;
if((_buddy->*_fun)(_key,_tval,_parameter))return true;
}
return false;
}
};

In my program, I have a lot of objects. I give each one a ID then put
their ptr in the map. Before using, I get it.

I am worry about the efficiency for the access is high frequency.
But I think the framework is clarity, and ID pass between function is
more clearer than object ptr.

 

[Ian Collins]

In my program, I have a lot of objects. I give each one a ID then put
their ptr in the map. Before using, I get it.

I am worry about the efficiency for the access is high frequency.
But I think the framework is clarity, and ID pass between function is
more clearer than object ptr.

But surely you can use the object pointer as a unique id?

 

[Darwin Lalo]

But surely you can use the object pointer as a unique id?

No. it is not a unique id.

int *p1 = new int;
delete p1;
int *p2 = new int;

(possibility) p1 == p2

But p1 is invaid. p2 is usable.

Yes, ref_count_ptr<int> can solve this problem.

 

[Ian Collins]

No. it is not a unique id.

int *p1 = new int;
delete p1;
int *p2 = new int;

(possibility) p1 == p2

Ah, you didn't say objects are deleted from the map.