@@ -251,14 +251,80 @@ void Arena_register(VALUE module) {
251251// The object is used only for its identity; it does not contain any data.
252252VALUE secondary_map = Qnil ;
253253
254+ // Mutations to the map are under a mutex, because SeconaryMap_MaybeGC()
255+ // iterates over the map which cannot happen in parallel with insertions, or
256+ // Ruby will throw:
257+ // can't add a new key into hash during iteration (RuntimeError)
258+ VALUE secondary_map_mutex = Qnil ;
259+
260+ // Lambda that will GC entries from the secondary map that are no longer present
261+ // in the primary map.
262+ VALUE gc_secondary_map_lambda = Qnil ;
263+ ID length ;
264+
265+ extern VALUE weak_obj_cache ;
266+
254267static void SecondaryMap_Init () {
255268 rb_gc_register_address (& secondary_map );
269+ rb_gc_register_address (& gc_secondary_map_lambda );
270+ rb_gc_register_address (& secondary_map_mutex );
256271 secondary_map = rb_hash_new ();
272+ gc_secondary_map_lambda = rb_eval_string (
273+ "->(secondary, weak) {\n"
274+ " secondary.delete_if { |k, v| !weak.key?(v) }\n"
275+ "}\n" );
276+ secondary_map_mutex = rb_mutex_new ();
277+ length = rb_intern ("length" );
257278}
258279
259- static VALUE SecondaryMap_Get (VALUE key ) {
280+ // The secondary map is a regular Hash, and will never shrink on its own.
281+ // The main object cache is a WeakMap that will automatically remove entries
282+ // when the target object is no longer reachable, but unless we manually
283+ // remove the corresponding entries from the secondary map, it will grow
284+ // without bound.
285+ //
286+ // To avoid this unbounded growth we periodically remove entries from the
287+ // secondary map that are no longer present in the WeakMap. The logic of
288+ // how often to perform this GC is an artbirary tuning parameter that
289+ // represents a straightforward CPU/memory tradeoff.
290+ //
291+ // Requires: secondary_map_mutex is held.
292+ static void SecondaryMap_MaybeGC () {
293+ PBRUBY_ASSERT (rb_mutex_locked_p (secondary_map_mutex ) == Qtrue );
294+ size_t weak_len = NUM2ULL (rb_funcall (weak_obj_cache , length , 0 ));
295+ size_t secondary_len = RHASH_SIZE (secondary_map );
296+ if (secondary_len < weak_len ) {
297+ // Logically this case should not be possible: a valid entry cannot exist in
298+ // the weak table unless there is a corresponding entry in the secondary
299+ // table. It should *always* be the case that secondary_len >= weak_len.
300+ //
301+ // However ObjectSpace::WeakMap#length (and therefore weak_len) is
302+ // unreliable: it overreports its true length by including non-live objects.
303+ // However these non-live objects are not yielded in iteration, so we may
304+ // have previously deleted them from the secondary map in a previous
305+ // invocation of SecondaryMap_MaybeGC().
306+ //
307+ // In this case, we can't measure any waste, so we just return.
308+ return ;
309+ }
310+ size_t waste = secondary_len - weak_len ;
311+ // GC if we could remove at least 2000 entries or 20% of the table size
312+ // (whichever is greater). Since the cost of the GC pass is O(N), we
313+ // want to make sure that we condition this on overall table size, to
314+ // avoid O(N^2) CPU costs.
315+ size_t threshold = PBRUBY_MAX (secondary_len * 0.2 , 2000 );
316+ if (waste > threshold ) {
317+ rb_funcall (gc_secondary_map_lambda , rb_intern ("call" ), 2 ,
318+ secondary_map , weak_obj_cache );
319+ }
320+ }
321+
322+ // Requires: secondary_map_mutex is held by this thread iff create == true.
323+ static VALUE SecondaryMap_Get (VALUE key , bool create ) {
324+ PBRUBY_ASSERT (!create || rb_mutex_locked_p (secondary_map_mutex ) == Qtrue );
260325 VALUE ret = rb_hash_lookup (secondary_map , key );
261- if (ret == Qnil ) {
326+ if (ret == Qnil && create ) {
327+ SecondaryMap_MaybeGC ();
262328 ret = rb_eval_string ("Object.new" );
263329 rb_hash_aset (secondary_map , key , ret );
264330 }
@@ -267,14 +333,15 @@ static VALUE SecondaryMap_Get(VALUE key) {
267333
268334#endif
269335
270- static VALUE ObjectCache_GetKey (const void * key ) {
336+ // Requires: secondary_map_mutex is held by this thread iff create == true.
337+ static VALUE ObjectCache_GetKey (const void * key , bool create ) {
271338 char buf [sizeof (key )];
272339 memcpy (& buf , & key , sizeof (key ));
273340 intptr_t key_int = (intptr_t )key ;
274341 PBRUBY_ASSERT ((key_int & 3 ) == 0 );
275342 VALUE ret = LL2NUM (key_int >> 2 );
276343#if USE_SECONDARY_MAP
277- ret = SecondaryMap_Get (ret );
344+ ret = SecondaryMap_Get (ret , create );
278345#endif
279346 return ret ;
280347}
@@ -298,14 +365,20 @@ static void ObjectCache_Init() {
298365
299366void ObjectCache_Add (const void * key , VALUE val ) {
300367 PBRUBY_ASSERT (ObjectCache_Get (key ) == Qnil );
301- VALUE key_rb = ObjectCache_GetKey (key );
368+ #if USE_SECONDARY_MAP
369+ rb_mutex_lock (secondary_map_mutex );
370+ #endif
371+ VALUE key_rb = ObjectCache_GetKey (key , true);
302372 rb_funcall (weak_obj_cache , item_set , 2 , key_rb , val );
373+ #if USE_SECONDARY_MAP
374+ rb_mutex_unlock (secondary_map_mutex );
375+ #endif
303376 PBRUBY_ASSERT (ObjectCache_Get (key ) == val );
304377}
305378
306379// Returns the cached object for this key, if any. Otherwise returns Qnil.
307380VALUE ObjectCache_Get (const void * key ) {
308- VALUE key_rb = ObjectCache_GetKey (key );
381+ VALUE key_rb = ObjectCache_GetKey (key , false );
309382 return rb_funcall (weak_obj_cache , item_get , 1 , key_rb );
310383}
311384
0 commit comments