Apply suggestions from code review

Co-authored-by: Dan Halbert <[email protected]>

Author: tannewt

ports/unix/mpconfigport.h

@@ -112,6 +112,7 @@ typedef long mp_off_t;
 
 // Always enable GC.
 #define MICROPY_ENABLE_GC           (1)
+// CIRCUITPY-CHANGE
 #define MICROPY_ENABLE_SELECTIVE_COLLECT (1)
 
 #if !(defined(MICROPY_GCREGS_SETJMP) || defined(__x86_64__) || defined(__i386__) || defined(__thumb2__) || defined(__thumb__) || defined(__arm__))

py/bc.h

@@ -302,13 +302,15 @@ static inline void mp_module_context_alloc_tables(mp_module_context_t *context,
     #if MICROPY_EMIT_BYTECODE_USES_QSTR_TABLE
     size_t nq = (n_qstr * sizeof(qstr_short_t) + sizeof(mp_uint_t) - 1) / sizeof(mp_uint_t);
     size_t no = n_obj;
+    // CIRCUITPY-CHANGE
     mp_uint_t *mem = m_malloc_items(nq + no);
     context->constants.qstr_table = (qstr_short_t *)mem;
     context->constants.obj_table = (mp_obj_t *)(mem + nq);
     #else
     if (n_obj == 0) {
         context->constants.obj_table = NULL;
     } else {
+        // CIRCUITPY-CHANGE
         context->constants.obj_table = m_malloc_items(n_obj);
     }
     #endif

py/compile.c

@@ -3689,6 +3689,7 @@ void mp_compile_to_raw_code(mp_parse_tree_t *parse_tree, qstr source_file, bool
 
 mp_obj_t mp_compile(mp_parse_tree_t *parse_tree, qstr source_file, bool is_repl) {
     mp_compiled_module_t cm;
+    // CIRCUITPY-CHANGE
     cm.context = m_malloc_with_collect(sizeof(mp_module_context_t));
     cm.context->module.globals = mp_globals_get();
     mp_compile_to_raw_code(parse_tree, source_file, is_repl, &cm);

py/emitbc.c

@@ -76,6 +76,7 @@ struct _emit_t {
 };
 
 emit_t *emit_bc_new(mp_emit_common_t *emit_common) {
+    // CIRCUITPY-CHANGE
     emit_t *emit = m_new_struct_with_collect(emit_t, 1);
     emit->emit_common = emit_common;
     return emit;

py/gc.c

@@ -331,7 +331,9 @@ static size_t compute_heap_size(size_t total_blocks) {
 static bool gc_try_add_heap(size_t failed_alloc) {
     // 'needed' is the size of a heap large enough to hold failed_alloc, with
     // the additional metadata overheads as calculated in gc_setup_area().
+    // CIRCUITPY-CHANGE: calculation of how much to grow the heap
     size_t total_new_blocks = (failed_alloc + BYTES_PER_BLOCK - 1) / BYTES_PER_BLOCK;
+    // CIRCUITPY-CHANGE
     size_t needed = compute_heap_size(total_new_blocks);
 
     size_t avail = gc_get_max_new_split();
@@ -376,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);
@@ -498,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);
@@ -1005,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();
@@ -1184,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);
     }
 
@@ -1329,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;
@@ -1343,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

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

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;

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);
 }
 

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) {

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.

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

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;

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,8 @@ static mp_obj_array_t *array_new(char typecode, size_t n) {
     o->typecode = typecode;
     o->free = 0;
     o->len = n;
+    // CIRCUITPY-CHANGE
+    // CIRCUITPY-CHANGE
     o->items = m_malloc(typecode_size * o->len);
     return o;
 }
@@ -227,6 +230,8 @@ 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
+    // 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 +691,8 @@ 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
+                // CIRCUITPY-CHANGE
                 res = mp_obj_malloc(mp_obj_array_t, &mp_type_memoryview);
                 *res = *o;
                 res->memview_offset += slice.start;

py/objclosure.c

@@ -50,6 +50,8 @@ 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
+        // 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));

py/objdeque.c

@@ -58,6 +58,8 @@ 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
+    // CIRCUITPY-CHANGE
     o->items = m_malloc_items(o->alloc);
 
     if (n_args > 2) {

py/objexcept.c

@@ -94,6 +94,8 @@ 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
+        // CIRCUITPY-CHANGE
         buf = m_malloc_with_collect(size);
     }
 
@@ -220,6 +222,8 @@ 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
+    // 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 +548,8 @@ 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
+        // 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 +667,8 @@ 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
+        // 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);

py/objtuple.c

@@ -86,6 +86,8 @@ 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
+            // CIRCUITPY-CHANGE
             mp_obj_t *items = m_malloc_items(alloc);
 
             mp_obj_t iterable = mp_getiter(args[0], NULL);

py/objtype.c

@@ -96,6 +96,8 @@ static mp_obj_t native_base_init_wrapper(size_t n_args, const mp_obj_t *pos_args
     pos_args++;
     n_args--;
 
+    // CIRCUITPY-CHANGE
+    // 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 +342,8 @@ 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
+            // 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 +375,8 @@ 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
+            // 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 +1519,8 @@ 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
+// 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);
 

py/parse.c

@@ -286,6 +286,8 @@ static void *parser_alloc(parser_t *parser, size_t num_bytes) {
         if (alloc < num_bytes) {
             alloc = num_bytes;
         }
+        // CIRCUITPY-CHANGE
+        // 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 +1057,8 @@ 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
+        // 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;

py/scope.c

@@ -51,6 +51,8 @@ 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
+    // CIRCUITPY-CHANGE
     scope_t *scope = m_new_struct_with_collect(scope_t, 1);
     scope->kind = kind;
     scope->pn = pn;

shared-bindings/displayio/Group.c

@@ -317,11 +317,14 @@ 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
+    // 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];
     }
     args[0] = MP_OBJ_FROM_PTR(self->members);
+    // CIRCUITPY-CHANGE
     mp_obj_t res = mp_obj_list_sort(n_args, args, kw_args);
     m_del(mp_obj_t, args, n_args);
     return res;

shared-module/fontio/BuiltinFont.c

@@ -16,6 +16,7 @@ mp_obj_t common_hal_fontio_builtinfont_get_bitmap(const fontio_builtinfont_t *se
 }
 
 mp_obj_t common_hal_fontio_builtinfont_get_bounding_box(const fontio_builtinfont_t *self) {
+    // CIRCUITPY-CHANGE
     // Stack allocation is ok because tuple copies the values out.
     mp_obj_t items[2];
     items[0] = MP_OBJ_NEW_SMALL_INT(self->width);