Apply suggestions from code review

tannewt · dhalbert · web-flow · commit a6dd85c09997 · 2025-04-24T10:46:47.000-07:00
Co-authored-by: Dan Halbert &lt;halbert@halwitz.org&gt;
diff --git a/py/gc.c b/py/gc.c
@@ -378,6 +378,7 @@ static bool gc_try_add_heap(size_t failed_alloc) {
         total_blocks += area->gc_alloc_table_byte_len * BLOCKS_PER_ATB;
     }
 
+    // CIRCUITPY-CHANGE
     size_t total_heap = compute_heap_size(total_blocks);
 
     DEBUG_printf("total_heap " UINT_FMT " bytes\n", total_heap);
@@ -500,6 +501,7 @@ static void MP_NO_INSTRUMENT PLACE_IN_ITCM(gc_mark_subtree)(size_t block)
         // check that the consecutive blocks didn't overflow past the end of the area
         assert(area->gc_pool_start + (block + n_blocks) * BYTES_PER_BLOCK <= area->gc_pool_end);
 
+        // CIRCUITPY-CHANGE
         // check if this block should be collected
         #if MICROPY_ENABLE_SELECTIVE_COLLECT
         bool should_scan = CTB_GET(area, block);
@@ -1007,6 +1009,7 @@ void *gc_alloc(size_t n_bytes, unsigned int alloc_flags) {
     (void)has_finaliser;
     #endif
 
+    // CIRCUITPY-CHANGE
     #if MICROPY_ENABLE_SELECTIVE_COLLECT
     bool do_not_collect = (alloc_flags & GC_ALLOC_FLAG_DO_NOT_COLLECT) != 0;
     GC_ENTER();
@@ -1186,6 +1189,7 @@ void *gc_realloc(void *ptr, mp_uint_t n_bytes) {
 void *gc_realloc(void *ptr_in, size_t n_bytes, bool allow_move) {
     // check for pure allocation
     if (ptr_in == NULL) {
+        // CIRCUITPY-CHANGE
         return gc_alloc(n_bytes, 0);
     }
 
@@ -1331,6 +1335,7 @@ void *gc_realloc(void *ptr_in, size_t n_bytes, bool allow_move) {
     }
     #endif
 
+    // CIRCUITPY-CHANGE
     #if MICROPY_ENABLE_SELECTIVE_COLLECT
     if (!CTB_GET(area, block)) {
         alloc_flags |= GC_ALLOC_FLAG_DO_NOT_COLLECT;
@@ -1345,6 +1350,7 @@ void *gc_realloc(void *ptr_in, size_t n_bytes, bool allow_move) {
     }
 
     // can't resize inplace; try to find a new contiguous chain
+    // CIRCUITPY-CHANGE
     void *ptr_out = gc_alloc(n_bytes, alloc_flags);
 
     // check that the alloc succeeded
diff --git a/py/gc.h b/py/gc.h
@@ -73,6 +73,7 @@ void gc_sweep_all(void);
 
 enum {
     GC_ALLOC_FLAG_HAS_FINALISER = 1,
+    // CIRCUITPY-CHANGE
     #if MICROPY_ENABLE_SELECTIVE_COLLECT
     GC_ALLOC_FLAG_DO_NOT_COLLECT = 2,
     #endif
diff --git a/py/lexer.c b/py/lexer.c
@@ -840,6 +840,7 @@ void mp_lexer_to_next(mp_lexer_t *lex) {
 }
 
 mp_lexer_t *mp_lexer_new(qstr src_name, mp_reader_t reader) {
+    // CIRCUITPY-CHANGE
     mp_lexer_t *lex = m_new_struct_with_collect(mp_lexer_t, 1);
 
     lex->source_name = src_name;
diff --git a/py/malloc.c b/py/malloc.c
@@ -119,6 +119,7 @@ void *m_malloc_helper(size_t num_bytes, uint8_t flags) {
     MP_STATE_MEM(current_bytes_allocated) += num_bytes;
     UPDATE_PEAK();
     #endif
+    // CIRCUITPY-CHANGE
     // If this config is set then the GC clears all memory, so we don't need to.
     #if !MICROPY_GC_CONSERVATIVE_CLEAR
     if (flags & M_MALLOC_ENSURE_ZEROED) {
@@ -130,10 +131,12 @@ void *m_malloc_helper(size_t num_bytes, uint8_t flags) {
 }
 
 void *m_malloc(size_t num_bytes) {
+    // CIRCUITPY-CHANGE
     return m_malloc_helper(num_bytes, M_MALLOC_RAISE_ERROR);
 }
 
 void *m_malloc_maybe(size_t num_bytes) {
+    // CIRCUITPY-CHANGE
     return m_malloc_helper(num_bytes, 0);
 }
 
@@ -142,10 +145,12 @@ void *m_malloc0(size_t num_bytes) {
 }
 
 void *m_malloc_with_collect(size_t num_bytes) {
+    // CIRCUITPY-CHANGE
     return m_malloc_helper(num_bytes, M_MALLOC_RAISE_ERROR | M_MALLOC_COLLECT);
 }
 
 void *m_malloc_maybe_with_collect(size_t num_bytes) {
+    // CIRCUITPY-CHANGE
     return m_malloc_helper(num_bytes, M_MALLOC_COLLECT);
 }
 
diff --git a/py/map.c b/py/map.c
@@ -95,6 +95,7 @@ void mp_map_init(mp_map_t *map, size_t n) {
         map->table = NULL;
     } else {
         map->alloc = n;
+        // CIRCUITPY-CHANGE
         map->table = malloc_table(map->alloc);
     }
     map->used = 0;
@@ -136,6 +137,7 @@ static void mp_map_rehash(mp_map_t *map) {
     size_t new_alloc = get_hash_alloc_greater_or_equal_to(map->alloc + 1);
     DEBUG_printf("mp_map_rehash(%p): " UINT_FMT " -> " UINT_FMT "\n", map, old_alloc, new_alloc);
     mp_map_elem_t *old_table = map->table;
+    // CIRCUITPY-CHANGE
     mp_map_elem_t *new_table = malloc_table(new_alloc);
     // If we reach this point, table resizing succeeded, now we can edit the old map.
     map->alloc = new_alloc;
@@ -332,6 +334,7 @@ mp_map_elem_t *MICROPY_WRAP_MP_MAP_LOOKUP(mp_map_lookup)(mp_map_t * map, mp_obj_
 void mp_set_init(mp_set_t *set, size_t n) {
     set->alloc = n;
     set->used = 0;
+    // CIRCUITPY-CHANGE
     set->table = m_malloc_items0(set->alloc);
 }
 
@@ -340,6 +343,7 @@ static void mp_set_rehash(mp_set_t *set) {
     mp_obj_t *old_table = set->table;
     set->alloc = get_hash_alloc_greater_or_equal_to(set->alloc + 1);
     set->used = 0;
+    // CIRCUITPY-CHANGE
     set->table = m_malloc_items0(set->alloc);
     for (size_t i = 0; i < old_alloc; i++) {
         if (old_table[i] != MP_OBJ_NULL && old_table[i] != MP_OBJ_SENTINEL) {
diff --git a/py/misc.h b/py/misc.h
@@ -74,6 +74,7 @@ typedef unsigned int uint;
 
 // TODO make a lazy m_renew that can increase by a smaller amount than requested (but by at least 1 more element)
 
+// CIRCUITPY-CHANGE: new wrappers for selective collect, and use of m_malloc_helper()
 // The following are convenience wrappers for m_malloc_helper and can save space at the call sites.
 // m_malloc and m_new allocate space that is not collected and does not have a finaliser.
 // Use m_malloc_items() to allocate space for mp_obj_t that will be collected.
diff --git a/py/mpstate.h b/py/mpstate.h
@@ -100,6 +100,7 @@ typedef struct _mp_state_mem_area_t {
     #if MICROPY_ENABLE_FINALISER
     byte *gc_finaliser_table_start;
     #endif
+    // CIRCUITPY-CHANGE
     #if MICROPY_ENABLE_SELECTIVE_COLLECT
     byte *gc_collect_table_start;
     #endif
diff --git a/py/obj.c b/py/obj.c
@@ -47,6 +47,7 @@
 
 // Allocates an object and also sets type, for mp_obj_malloc{,_var} macros.
 MP_NOINLINE void *mp_obj_malloc_helper(size_t num_bytes, const mp_obj_type_t *type) {
+    // CIRCUITPY-CHANGE
     mp_obj_base_t *base = (mp_obj_base_t *)m_malloc_helper(num_bytes, M_MALLOC_RAISE_ERROR | M_MALLOC_COLLECT);
     base->type = type;
     return base;
@@ -55,6 +56,7 @@ MP_NOINLINE void *mp_obj_malloc_helper(size_t num_bytes, const mp_obj_type_t *ty
 #if MICROPY_ENABLE_FINALISER
 // Allocates an object and also sets type, for mp_obj_malloc{,_var}_with_finaliser macros.
 MP_NOINLINE void *mp_obj_malloc_with_finaliser_helper(size_t num_bytes, const mp_obj_type_t *type) {
+    // CIRCUITPY-CHANGE
     mp_obj_base_t *base = (mp_obj_base_t *)m_malloc_helper(num_bytes, M_MALLOC_RAISE_ERROR | M_MALLOC_COLLECT | M_MALLOC_WITH_FINALISER);
     base->type = type;
     return base;
diff --git a/py/objarray.c b/py/objarray.c
@@ -106,6 +106,7 @@ static mp_obj_array_t *array_new(char typecode, size_t n) {
     }
     int typecode_size = mp_binary_get_size('@', typecode, NULL);
 
+    // CIRCUITPY-CHANGE: refactor to use m_obj_malloc()
     const mp_obj_type_t *type;
     #if MICROPY_PY_BUILTINS_BYTEARRAY && MICROPY_PY_ARRAY
     type = (typecode == BYTEARRAY_TYPECODE) ? &mp_type_bytearray : &mp_type_array;
@@ -118,6 +119,7 @@ static mp_obj_array_t *array_new(char typecode, size_t n) {
     o->typecode = typecode;
     o->free = 0;
     o->len = n;
+    // CIRCUITPY-CHANGE
     o->items = m_malloc(typecode_size * o->len);
     return o;
 }
@@ -227,6 +229,7 @@ static mp_obj_t bytearray_make_new(const mp_obj_type_t *type_in, size_t n_args,
 #if MICROPY_PY_BUILTINS_MEMORYVIEW
 
 mp_obj_t mp_obj_new_memoryview(byte typecode, size_t nitems, void *items) {
+    // CIRCUITPY-CHANGE
     mp_obj_array_t *self = mp_obj_malloc(mp_obj_array_t, &mp_type_memoryview);
     mp_obj_memoryview_init(self, typecode, 0, nitems, items);
     return MP_OBJ_FROM_PTR(self);
@@ -686,6 +689,7 @@ static mp_obj_t array_subscr(mp_obj_t self_in, mp_obj_t index_in, mp_obj_t value
                 if (slice.start > memview_offset_max) {
                     mp_raise_msg(&mp_type_OverflowError, MP_ERROR_TEXT("memoryview offset too large"));
                 }
+                // CIRCUITPY-CHANGE
                 res = mp_obj_malloc(mp_obj_array_t, &mp_type_memoryview);
                 *res = *o;
                 res->memview_offset += slice.start;
diff --git a/py/objclosure.c b/py/objclosure.c
@@ -50,6 +50,7 @@ static mp_obj_t closure_call(mp_obj_t self_in, size_t n_args, size_t n_kw, const
         return mp_call_function_n_kw(self->fun, self->n_closed + n_args, n_kw, args2);
     } else {
         // use heap to allocate temporary args array
+        // CIRCUITPY-CHANGE
         mp_obj_t *args2 = m_malloc_items(n_total);
         memcpy(args2, self->closed, self->n_closed * sizeof(mp_obj_t));
         memcpy(args2 + self->n_closed, args, (n_args + 2 * n_kw) * sizeof(mp_obj_t));
diff --git a/py/objdeque.c b/py/objdeque.c
@@ -58,6 +58,7 @@ static mp_obj_t deque_make_new(const mp_obj_type_t *type, size_t n_args, size_t
     mp_obj_deque_t *o = mp_obj_malloc(mp_obj_deque_t, type);
     o->alloc = maxlen + 1;
     o->i_get = o->i_put = 0;
+    // CIRCUITPY-CHANGE
     o->items = m_malloc_items(o->alloc);
 
     if (n_args > 2) {
diff --git a/py/objexcept.c b/py/objexcept.c
@@ -94,6 +94,7 @@ mp_obj_t mp_alloc_emergency_exception_buf(mp_obj_t size_in) {
     mp_int_t size = mp_obj_get_int(size_in);
     void *buf = NULL;
     if (size > 0) {
+        // CIRCUITPY-CHANGE
         buf = m_malloc_with_collect(size);
     }
 
@@ -220,6 +221,7 @@ mp_obj_t mp_obj_exception_make_new(const mp_obj_type_t *type, size_t n_args, siz
     mp_arg_check_num(n_args, n_kw, 0, MP_OBJ_FUN_ARGS_MAX, false);
 
     // Try to allocate memory for the exception, with fallback to emergency exception object
+    // CIRCUITPY-CHANGE
     mp_obj_exception_t *o_exc = m_malloc_maybe_with_collect(sizeof(mp_obj_exception_t));
     if (o_exc == NULL) {
         o_exc = &MP_STATE_VM(mp_emergency_exception_obj);
@@ -544,6 +546,7 @@ mp_obj_t mp_obj_new_exception_msg_vlist(const mp_obj_type_t *exc_type, mp_rom_er
     // CIRCUITPY-CHANGE: here and more below
     size_t o_str_alloc = decompress_length(fmt);
     if (gc_alloc_possible()) {
+        // CIRCUITPY-CHANGE
         o_str = m_malloc_maybe_with_collect(sizeof(mp_obj_str_t));
         o_str_buf = m_new_maybe(byte, o_str_alloc);
     }
@@ -661,6 +664,7 @@ void mp_obj_exception_add_traceback(mp_obj_t self_in, qstr file, size_t line, qs
 
     // Try to allocate memory for the traceback, with fallback to emergency traceback object
     if (self->traceback == NULL || self->traceback == (mp_obj_traceback_t *)&mp_const_empty_traceback_obj) {
+        // CIRCUITPY-CHANGE
         self->traceback = m_malloc_maybe_with_collect(sizeof(mp_obj_traceback_t));
         if (self->traceback == NULL) {
             self->traceback = &MP_STATE_VM(mp_emergency_traceback_obj);
diff --git a/py/objtuple.c b/py/objtuple.c
@@ -86,6 +86,7 @@ static mp_obj_t mp_obj_tuple_make_new(const mp_obj_type_t *type_in, size_t n_arg
 
             size_t alloc = 4;
             size_t len = 0;
+            // CIRCUITPY-CHANGE
             mp_obj_t *items = m_malloc_items(alloc);
 
             mp_obj_t iterable = mp_getiter(args[0], NULL);
diff --git a/py/objtype.c b/py/objtype.c
@@ -96,6 +96,7 @@ static mp_obj_t native_base_init_wrapper(size_t n_args, const mp_obj_t *pos_args
     pos_args++;
     n_args--;
 
+    // CIRCUITPY-CHANGE
     mp_obj_t *args2 = m_malloc_items(n_args + 2 * n_kw);
     // copy in args
     memcpy(args2, pos_args, n_args * sizeof(mp_obj_t));
@@ -340,6 +341,7 @@ static mp_obj_t mp_obj_instance_make_new(const mp_obj_type_t *self, size_t n_arg
             mp_obj_t args2[1] = {MP_OBJ_FROM_PTR(self)};
             new_ret = mp_call_function_n_kw(init_fn[0], 1, 0, args2);
         } else {
+            // CIRCUITPY-CHANGE
             mp_obj_t *args2 = m_malloc_items(1 + n_args + 2 * n_kw);
             args2[0] = MP_OBJ_FROM_PTR(self);
             memcpy(args2 + 1, args, (n_args + 2 * n_kw) * sizeof(mp_obj_t));
@@ -371,6 +373,7 @@ static mp_obj_t mp_obj_instance_make_new(const mp_obj_type_t *self, size_t n_arg
         if (n_args == 0 && n_kw == 0) {
             init_ret = mp_call_method_n_kw(0, 0, init_fn);
         } else {
+            // CIRCUITPY-CHANGE
             mp_obj_t *args2 = m_malloc_items(2 + n_args + 2 * n_kw);
             args2[0] = init_fn[0];
             args2[1] = init_fn[1];
@@ -1513,6 +1516,7 @@ mp_obj_t mp_obj_cast_to_native_base(mp_obj_t self_in, mp_const_obj_t native_type
 /******************************************************************************/
 // staticmethod and classmethod types (probably should go in a different file)
 
+// CIRCUITPY-CHANGE: better arg name
 static mp_obj_t static_class_method_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *args) {
     assert(type == &mp_type_staticmethod || type == &mp_type_classmethod);
 
diff --git a/py/parse.c b/py/parse.c
@@ -286,6 +286,7 @@ static void *parser_alloc(parser_t *parser, size_t num_bytes) {
         if (alloc < num_bytes) {
             alloc = num_bytes;
         }
+        // CIRCUITPY-CHANGE
         chunk = (mp_parse_chunk_t *)m_malloc_with_collect(sizeof(mp_parse_chunk_t) + alloc);
         chunk->alloc = alloc;
         chunk->union_.used = 0;
@@ -1055,6 +1056,7 @@ mp_parse_tree_t mp_parse(mp_lexer_t *lex, mp_parse_input_kind_t input_kind) {
     parser.result_stack_top = 0;
     parser.result_stack = NULL;
     while (parser.result_stack_alloc > 1) {
+        // CIRCUITPY-CHANGE
         parser.result_stack = m_malloc_maybe_with_collect(sizeof(mp_parse_node_t) * parser.result_stack_alloc);
         if (parser.result_stack != NULL) {
             break;
diff --git a/py/scope.c b/py/scope.c
@@ -51,6 +51,7 @@ scope_t *scope_new(scope_kind_t kind, mp_parse_node_t pn, mp_uint_t emit_options
     MP_STATIC_ASSERT(MP_QSTR__lt_setcomp_gt_ <= UINT8_MAX);
     MP_STATIC_ASSERT(MP_QSTR__lt_genexpr_gt_ <= UINT8_MAX);
 
+    // CIRCUITPY-CHANGE
     scope_t *scope = m_new_struct_with_collect(scope_t, 1);
     scope->kind = kind;
     scope->pn = pn;
diff --git a/shared-bindings/displayio/Group.c b/shared-bindings/displayio/Group.c
@@ -317,6 +317,7 @@ static mp_obj_t group_subscr(mp_obj_t self_in, mp_obj_t index_obj, mp_obj_t valu
 //|
 static mp_obj_t displayio_group_obj_sort(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
     displayio_group_t *self = native_group(pos_args[0]);
+    // CIRCUITPY-CHANGE
     mp_obj_t *args = m_malloc_items(n_args);
     for (size_t i = 1; i < n_args; ++i) {
         args[i] = pos_args[i];

Original file line number	Diff line number	Diff line change
`@@ -840,6 +840,7 @@ void mp_lexer_to_next(mp_lexer_t *lex) {`
`840`	`840`	`}`
`841`	`841`
`842`	`842`	`mp_lexer_t *mp_lexer_new(qstr src_name, mp_reader_t reader) {`
	`843`	`+ // CIRCUITPY-CHANGE`
`843`	`844`	`mp_lexer_t *lex = m_new_struct_with_collect(mp_lexer_t, 1);`
`844`	`845`
`845`	`846`	`lex->source_name = src_name;`
Original file line number	Diff line number	Diff line change
`@@ -119,6 +119,7 @@ void *m_malloc_helper(size_t num_bytes, uint8_t flags) {`
`119`	`119`	`MP_STATE_MEM(current_bytes_allocated) += num_bytes;`
`120`	`120`	`UPDATE_PEAK();`
`121`	`121`	`#endif`
	`122`	`+ // CIRCUITPY-CHANGE`
`122`	`123`	`// If this config is set then the GC clears all memory, so we don't need to.`
`123`	`124`	`#if !MICROPY_GC_CONSERVATIVE_CLEAR`
`124`	`125`	`if (flags & M_MALLOC_ENSURE_ZEROED) {`
`@@ -130,10 +131,12 @@ void *m_malloc_helper(size_t num_bytes, uint8_t flags) {`
`130`	`131`	`}`
`131`	`132`
`132`	`133`	`void *m_malloc(size_t num_bytes) {`
	`134`	`+ // CIRCUITPY-CHANGE`
`133`	`135`	`return m_malloc_helper(num_bytes, M_MALLOC_RAISE_ERROR);`
`134`	`136`	`}`
`135`	`137`
`136`	`138`	`void *m_malloc_maybe(size_t num_bytes) {`
	`139`	`+ // CIRCUITPY-CHANGE`
`137`	`140`	`return m_malloc_helper(num_bytes, 0);`
`138`	`141`	`}`
`139`	`142`
`@@ -142,10 +145,12 @@ void *m_malloc0(size_t num_bytes) {`
`142`	`145`	`}`
`143`	`146`
`144`	`147`	`void *m_malloc_with_collect(size_t num_bytes) {`
	`148`	`+ // CIRCUITPY-CHANGE`
`145`	`149`	`return m_malloc_helper(num_bytes, M_MALLOC_RAISE_ERROR \| M_MALLOC_COLLECT);`
`146`	`150`	`}`
`147`	`151`
`148`	`152`	`void *m_malloc_maybe_with_collect(size_t num_bytes) {`
	`153`	`+ // CIRCUITPY-CHANGE`
`149`	`154`	`return m_malloc_helper(num_bytes, M_MALLOC_COLLECT);`
`150`	`155`	`}`
`151`	`156`