Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unbounded memory growth for Ruby <2.7. #8429

Merged
merged 4 commits into from Mar 29, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 49 additions & 0 deletions ruby/ext/google/protobuf_c/protobuf.c
Expand Up @@ -251,16 +251,65 @@ void Arena_register(VALUE module) {
// The object is used only for its identity; it does not contain any data.
VALUE secondary_map = Qnil;

// Mutations to the map are under a mutex, because SeconaryMap_MaybeGC()
// iterates over the map which cannot happen in parallel with insertions, or
// Ruby will throw:
// can't add a new key into hash during iteration (RuntimeError)
VALUE secondary_map_mutex = Qnil;

// Lambda that will GC entries from the secondary map that are no longer present
// in the primary map.
VALUE gc_secondary_map = Qnil;
ID length;

extern VALUE weak_obj_cache;

static void SecondaryMap_Init() {
rb_gc_register_address(&secondary_map);
rb_gc_register_address(&gc_secondary_map);
rb_gc_register_address(&secondary_map_mutex);
secondary_map = rb_hash_new();
gc_secondary_map = rb_eval_string(
haberman marked this conversation as resolved.
Show resolved Hide resolved
"->(secondary, weak) {\n"
" secondary.delete_if { |k, v| !weak.key?(v) }\n"
"}\n");
secondary_map_mutex = rb_mutex_new();
length = rb_intern("length");
}

// The secondary map is a regular Hash, and will never shrink on its own.
// The main object cache is a WeakMap that will automatically remove entries
// when the target object is no longer reachable, but unless we manually
// remove the corresponding entries from the secondary map, it will grow
// without bound.
//
// To avoid this unbounded growth we periodically remove entries from the
// secondary map that are no longer present in the WeakMap. The logic of
// how often to perform this GC is an artbirary tuning parameter that
// represents a straightforward CPU/memory tradeoff.
static void SecondaryMap_MaybeGC() {
haberman marked this conversation as resolved.
Show resolved Hide resolved
size_t weak_len = NUM2ULL(rb_funcall(weak_obj_cache, length, 0));
size_t secondary_len = RHASH_SIZE(secondary_map);
size_t waste = secondary_len - weak_len;
PBRUBY_ASSERT(secondary_len >= weak_len);
// GC if we could remove at least 2000 entries or 20% of the table size
// (whichever is greater). Since the cost of the GC pass is O(N), we
// want to make sure that we condition this on overall table size, to
// avoid O(N^2) CPU costs.
size_t threshold = PBRUBY_MAX(secondary_len * 0.2, 2000);
if (waste > threshold) {
rb_funcall(gc_secondary_map, rb_intern("call"), 2, secondary_map, weak_obj_cache);
}
}

static VALUE SecondaryMap_Get(VALUE key) {
VALUE ret = rb_hash_lookup(secondary_map, key);
if (ret == Qnil) {
rb_mutex_lock(secondary_map_mutex);
SecondaryMap_MaybeGC();
ret = rb_eval_string("Object.new");
rb_hash_aset(secondary_map, key, ret);
rb_mutex_unlock(secondary_map_mutex);
}
return ret;
}
Expand Down
2 changes: 2 additions & 0 deletions ruby/ext/google/protobuf_c/protobuf.h
Expand Up @@ -106,6 +106,8 @@ extern VALUE cTypeError;
#define PBRUBY_ASSERT(expr) assert(expr)
#endif

#define PBRUBY_MAX(x, y) (((x) > (y)) ? (x) : (y))

#define UPB_UNUSED(var) (void)var

#endif // __GOOGLE_PROTOBUF_RUBY_PROTOBUF_H__