It looks like I introduced a memory leak in https://github.com/arthurnn/memcached/pull/199 from misunderstanding the purpose of rb_external_str_new_with_enc. It turns out that it just copies the external string contents into the ruby string, rather than taking ownership of the external string and freeing it. So it both introduces a memory leak and doesn't avoid the extra copy.
Solution
One use of rb_str_new_by_ref was used to implement Memcached#get_from_last, which is deprecated, so I'm not going to worry about avoiding the copy for that code. We can just use rb_str_new and free as was previously done for Rubinius and JRuby.
I would like to avoid a performance regression for Memcached#get, which is the other place rb_str_new_by_ref was used. However, I found there was already a needless copy in the implementation of memcached_fetch that I was able to get rid of by using memcached_fetch_result. I added that optimization for the multi-get case and just delegated to that for single gets.
Problem
It looks like I introduced a memory leak in https://github.com/arthurnn/memcached/pull/199 from misunderstanding the purpose of rb_external_str_new_with_enc. It turns out that it just copies the external string contents into the ruby string, rather than taking ownership of the external string and freeing it. So it both introduces a memory leak and doesn't avoid the extra copy.
Solution
One use of
rb_str_new_by_refwas used to implementMemcached#get_from_last, which is deprecated, so I'm not going to worry about avoiding the copy for that code. We can just userb_str_newandfreeas was previously done for Rubinius and JRuby.I would like to avoid a performance regression for
Memcached#get, which is the other placerb_str_new_by_refwas used. However, I found there was already a needless copy in the implementation ofmemcached_fetchthat I was able to get rid of by usingmemcached_fetch_result. I added that optimization for the multi-get case and just delegated to that for single gets.