From 73771e4b192f3db62efb854affdfc95babba1d35 Mon Sep 17 00:00:00 2001
From: Jean Boussier <byroot@ruby-lang.org>
Date: Tue, 6 Dec 2022 12:56:51 +0100
Subject: ObjectSpace.dump_all: dump shapes as well

I see several arguments in doing so.

First they use a non trivial amount of memory, so for various memory
profiling/mapping tools it is relevant to have visibility of the space
occupied by shapes.

Then, some pathological code can create a tons of shape, so it is
valuable to have a way to have a way to observe shapes without having
to compile Ruby with `SHAPE_DEBUG=1`.

And additionally it's likely much faster to dump then this way than
to use `RubyVM::Shape`.

There are however a few open questions:

- Shapes can't respect the `since:` argument. Not sure what to do when
  it is provided. Would probably make sense to not dump them.
- Maybe it would make more sense to have a separate `ObjectSpace.dump_shapes`?
- Maybe instead `dump_all` should take a `shapes: false` argument?

Additionally, `ObjectSpace.dump_shapes` is added for the use case of
debugging the evolution of the shape tree.
---
 shape.c | 79 +++++++++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 55 insertions(+), 24 deletions(-)

(limited to 'shape.c')

diff --git a/shape.c b/shape.c
index 973a8a6328..41d3cfcf84 100644
--- a/shape.c
+++ b/shape.c
@@ -2,6 +2,8 @@
 #include "vm_sync.h"
 #include "shape.h"
 #include "gc.h"
+#include "symbol.h"
+#include "id_table.h"
 #include "internal/class.h"
 #include "internal/symbol.h"
 #include "internal/variable.h"
@@ -37,6 +39,17 @@ rb_shape_root_shape_p(rb_shape_t* shape)
     return shape == rb_shape_get_root_shape();
 }
 
+void
+rb_shape_each_shape(each_shape_callback callback, void *data)
+{
+    rb_shape_t *cursor = rb_shape_get_root_shape();
+    rb_shape_t *end = rb_shape_get_shape_by_id(GET_VM()->next_shape_id);
+    while (cursor < end) {
+        callback(cursor, data);
+        cursor += 1;
+    }
+}
+
 rb_shape_t*
 rb_shape_get_shape_by_id(shape_id_t shape_id)
 {
@@ -97,10 +110,10 @@ rb_shape_get_shape_id(VALUE obj)
 #endif
 }
 
-unsigned int
+size_t
 rb_shape_depth(rb_shape_t * shape)
 {
-    unsigned int depth = 1;
+    size_t depth = 1;
 
     while (shape->parent_id != INVALID_SHAPE_ID) {
         depth++;
@@ -285,6 +298,7 @@ rb_shape_transition_shape_frozen(VALUE obj)
 rb_shape_t *
 rb_shape_get_next_iv_shape(rb_shape_t* shape, ID id)
 {
+    RUBY_ASSERT(!is_instance_id(id) || RTEST(rb_sym2str(ID2SYM(id))));
     return get_next_shape_internal(shape, id, SHAPE_IVAR);
 }
 
@@ -428,16 +442,45 @@ rb_shape_rebuild_shape(rb_shape_t * initial_shape, rb_shape_t * dest_shape)
     return midway_shape;
 }
 
+size_t
+rb_shape_edges_count(rb_shape_t *shape)
+{
+    if (shape->edges) {
+        return rb_id_table_size(shape->edges);
+    }
+    return 0;
+}
+
+size_t
+rb_shape_memsize(rb_shape_t *shape)
+{
+    size_t memsize = sizeof(rb_shape_t);
+    if (shape->edges) {
+        memsize += rb_id_table_memsize(shape->edges);
+    }
+    return memsize;
+}
+
 #if SHAPE_DEBUG
 VALUE rb_cShape;
 
+static size_t
+shape_memsize(const void *shape_ptr)
+{
+    return rb_shape_memsize((rb_shape_t *)shape_ptr);
+}
+
 /*
  * Exposing Shape to Ruby via RubyVM.debug_shape
  */
 static const rb_data_type_t shape_data_type = {
-    "Shape",
-    {NULL, NULL, NULL,},
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY|RUBY_TYPED_WB_PROTECTED
+    .wrap_struct_name = "Shape",
+    .function = {
+        .dmark = NULL,
+        .dfree = NULL,
+        .dsize = shape_memsize,
+    },
+    .flags = RUBY_TYPED_FREE_IMMEDIATELY|RUBY_TYPED_WB_PROTECTED
 };
 
 static VALUE
@@ -480,12 +523,10 @@ rb_shape_parent_id(VALUE self)
 static VALUE
 parse_key(ID key)
 {
-    if ((key & RUBY_ID_INTERNAL) == RUBY_ID_INTERNAL) {
-        return LONG2NUM(key);
-    }
-    else {
+    if (is_instance_id(key)) {
         return ID2SYM(key);
     }
+    return LONG2NUM(key);
 }
 
 static VALUE
@@ -527,17 +568,13 @@ rb_shape_edge_name(VALUE self)
     rb_shape_t* shape;
     TypedData_Get_Struct(self, rb_shape_t, &shape_data_type, shape);
 
-    if ((shape->edge_name & (ID_INTERNAL)) == ID_INTERNAL) {
-        return INT2NUM(shape->capacity);
-    }
-    else {
-        if (shape->edge_name) {
+    if (shape->edge_name) {
+        if (is_instance_id(shape->edge_name)) {
             return ID2SYM(shape->edge_name);
         }
-        else {
-            return Qnil;
-        }
+        return INT2NUM(shape->capacity);
     }
+    return Qnil;
 }
 
 static VALUE
@@ -563,13 +600,7 @@ rb_shape_export_depth(VALUE self)
 {
     rb_shape_t* shape;
     TypedData_Get_Struct(self, rb_shape_t, &shape_data_type, shape);
-
-    unsigned int depth = 0;
-    while (shape->parent_id != INVALID_SHAPE_ID) {
-        depth++;
-        shape = rb_shape_get_parent(shape);
-    }
-    return INT2NUM(depth);
+    return SIZET2NUM(rb_shape_depth(shape));
 }
 
 static VALUE
-- 
cgit v1.2.1