Re: Garbage Collection - The Trash Begins To Pile Up

Andreas Huber wrote:


The problem with this code:

void C::update(boost::shared_ptr<has_foo> other) {
  boost::mutex::scoped_lock scoped_lock(mutex);
  int count = impl_use_count[my_index];
  long new_val = combine(impls[my_index] -> get_data(),
               X::messy_fn(other));
  if (count > 1) {
    // Clone my C_impl.
      int new_index = first_available();
      impl_use_count[new_index] = 1;
      impls[new_index] = new C_impl(new_val);
      --impl_use_count[my_index];
      my_index = new_index;
   } else {
      impls[my_index]->set_data(new_val);
   }
}

is that it calls a "messy function" while holding a lock. It's true
that calling the destructor asynchronously (or not calling it at all in
this case since GC will take care of the impls) can prevent some of the
problems with this approach. But certainly not all. A simple
illustration is

long X::messy_fn( x )
{
  C c;
  return 0;
}

In addition, holding the class level C lock while waiting for
X::messy_fn is a performance problem as well. It can easily negate any
implementation sharing/caching gains. Copy on write doesn't play well
with threads, too, and in this case is not a guaranteed win since it
clobbers the C_impl holding the old value and doesn't probe the cache
for new_val. Why not just

void C::update(boost::shared_ptr<has_foo> other)
{
  long new_val = combine(pimpl_ -> get_data(),
               X::messy_fn(other));
  pimpl_ = create_C_impl( new_val );
}

where pimpl_ is a shared_ptr<C_impl> and create_C_impl is something
like

shared_ptr<C_impl> create_C_impl( long value ) synchronized
{
  if( cache contains value )
  {
    return cached impl;
  }
  else
  {
    shared_ptr<C_impl> pci( new C_impl( value ) );
    update_cache_with( pci ); // depending on cache policy
    return pci;
  }
}

--
      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated. First time posters: Do this! ]