Working with class.

Issue #283 invalid
Ngoc Phuong Chau created an issue

I wanna create a class as below

class myclass
{
  public:
      int k;
      upcxx::global_ptr<double> vector; 
      myclass(int n);
      ~myclass();
};
myclass::myclass(int n)
{
  this->k=0;
  this-> vector = upcxx::new_array<double>(n);
  double* local_vec = vector.local();
  for(int i=0 ;i < n; i++)
  {
    local_vec[i]=1;
  }
}
myclass::~myclass()
{
  upcxx::delete_array(this->vector);
}

Then, I broadcast to other ranks.

upcxx::global_ptr<myclass> *all_elements= new upcxx::global_ptr<myclass> [m];

For single machine, I used “local() function to downcast the pointer to work“; it worked.

However, with 2 or more machines, I used “rget()” to get global pointer from other teams, and the error is

In instantiation of ‘typename upcxx::detail::completions_returner<upcxx::detail::event_is_here, upcxx::detail::rget_byval_event_values<T>, Cxs>::return_t upcxx::rget(upcxx::global_ptr<T>, Cxs) [with T = myclass; Cxs = upcxx::completions<upcxx::future_cx<upcxx::operation_cx_event, (upcxx::progress_level)1> >; typename upcxx::detail::completions_returner<upcxx::detail::event_is_here, upcxx::detail::rget_byval_event_values<T>, Cxs>::return_t = upcxx::future1<upcxx::detail::future_kind_shrefupcxx::detail::future_header_ops_result, myclass>]’:
test_object.cpp:108:47: required from here
/cluster/home/cnphuong/upcxx/upcxx.O3.gasnet_seq.ibv/include/upcxx/rget.hpp:181:5: error: static assertion failed: RMA operations only work on DefinitelyTriviallySerializable types.

It also cannot work when I change global pointer in the class (vector) to vector<int> or int* (array).

I read the document and see that global_ptr<T>. T is DefinitelyTriviallySerializable.

Could you please recommend some method to work in this case? I think we can use RPC; however, I store a lot of data in local, if I use RPC the cost for communication is very big.

Thanks.

Comments (3)

  1. Dan Bonachea

    Hi Ngoc -

    As the error message states, RMA operations (rget) only work for DefinitelyTriviallySerializable types (which usually means TriviallyCopyable). This is because RMA performs a trivial copy of the object.

    In your case your class is not TriviallyCopyable, because of the destructor. If RMA copied an object of myclass to another process, then upon destruction of that object it would attempt to delete_array on the remote shared object, which is prohibited. You can fix this defect in the destructor by checking this->vector.where() == upcxx::rank_me() before calling delete_array, although this is still potentially "dangerous" because it means the first local copy of the object destroys the vector that might still be referenced by other copies.

    Note that even ignoring remote processes, this class won't work as expected if any instances are copied, because the destructors act like they own the vector state but the compiler-generated implicit copy constructor shares that state with copies. For C++ cleanliness you should either implement "real" versions of the copy constructor and assignment that deep-clone the vector, or delete the implicit compiler-generated ones to prohibit unsafe copies. (You should probably also ensure that move constructor/assignment is correctly handled, but that's not directly relevant here)

    None of these gets you closer to a class that works with RMA, which always performs a trivial copy. If you want you can force UPC++ to treat the class as DefinitelyTriviallySerializable by specializing upcxx::is_definitely_trivially_serializable<myclass>::value to true - this enables RMA but doesn't change the fact it's a trivial copy so you still might encounter dangerous behavior from the destructor.

    You have not explained the context of the code using this class, but (depending on how you resolve the copying issue) it might make more sense to move the resource reclamation out of the destructor into an explicit destroy method that the client ensures is explicitly called once per vector, before the last copy is destroyed.

    Hope this helps..

  2. Log in to comment