charmplusplus / charm

The Charm++ parallel programming system. Visit https://charmplusplus.org/ for more information.
Apache License 2.0
208 stars 49 forks source link

improve efficiency of exclusive entry methods #858

Open jcphill opened 9 years ago

jcphill commented 9 years ago

Original issue: https://charm.cs.illinois.edu/redmine/issues/858


From tests/charm++/pingpong/pingpong.def.h:

 void CkIndex_PingN::_call_recvRDMA_void(void* impl_msg, void* impl_obj_void)
 {
  PingN* impl_obj = static_cast<PingN *>(impl_obj_void);
  if(CmiTryLock(impl_obj->__nodelock)) {
    CkSendMsgNodeBranch(CkIndex_PingN::idx_recvRDMA_void(),impl_msg,CkMyNode(),impl_obj->CkGetNodeGroupID());
    return;
  }
  CkFreeSysMsg(impl_msg);
  impl_obj->recvRDMA();
  CmiUnlock(impl_obj->__nodelock);
 }

The CkFreeSysMsg() call should be moved after CmiUnlock().

Of course, this only fixes exclusive entries without arguments, since an entry taking a message is responsible for deleting that message, which probably happens inside the lock.

Marshalled parameters are even worse, since they copy the message if they can't get a lock because they are considered nokeep:

 void CkIndex_PingN::_call_recvHandle_marshall6(void* impl_msg, void* impl_obj_void)
 {
  PingN* impl_obj = static_cast<PingN *>(impl_obj_void);
  if(CmiTryLock(impl_obj->__nodelock)) {
    impl_msg = CkCopyMsg(&impl_msg);
    CkSendMsgNodeBranch(CkIndex_PingN::idx_recvHandle_marshall6(),impl_msg,CkMyNode(),impl_obj->CkGetNodeGroupID());
    return;
  }
  ...unmarshall parameters...
  impl_obj->recvHandle(ptr, size);
  CmiUnlock(impl_obj->__nodelock);
 }

The larger issue is that the message will be repeatedly resubmitted until the lock is free. It would be better to attach a message queue to the nodegroup object to hold all exclusive messages that arrive while the lock is held, and then have the pe holding the lock re-enqueue only the single highest-priority exclusive message.

stwhite91 commented 5 years ago

Original date: 2016-11-02 23:49:22


Moving this to Nitin since he has more experience with charmxi. I think it should be easy to do what Jim has pointed out for exclusive entry methods that take no arguments, and the more complicated cases we can work out later.

stwhite91 commented 5 years ago

Original date: 2017-01-30 00:41:48


Some progress on at least the simple case here before the 6.8.0 release would be valuable.

PhilMiller commented 5 years ago

Original date: 2017-01-30 16:29:50


In the parameter marshalling case, it seems like CmiReference(impl_msg) would suffice to balance out the deletion that will happen in the calling code. Then the original message can be queued up, rather than a copy.

kavithachandrasekar commented 5 years ago

Original date: 2017-12-06 20:11:28


This commit https://charm.cs.illinois.edu/gerrit/#/c/3344/ https://github.com/UIUC-PPL/charm/commit/16a0ba8b11824cac157c0a6a353a842ace9b4f03 is a fix for exclusive entry methods without arguments.