Merge pull request adafruit#985 from tannewt/heap_tweaks3

dhalbert · web-flow · commit 64b9ee9c74dd · 2018-07-08T23:21:38.000-04:00
A few heap related tweaks
diff --git a/.gitignore b/.gitignore
@@ -58,3 +58,4 @@ TAGS
 #################
 *.orig
 
+*.DS_Store
diff --git a/lib/utils/pyexec.c b/lib/utils/pyexec.c
@@ -33,6 +33,7 @@
 #include "py/runtime.h"
 #include "py/repl.h"
 #include "py/gc.h"
+#include "py/gc_long_lived.h"
 #include "py/frozenmod.h"
 #include "py/mphal.h"
 #if defined(USE_DEVICE_MODE)
@@ -97,6 +98,13 @@ STATIC int parse_compile_execute(const void *source, mp_parse_input_kind_t input
             #endif
         }
 
+        // If the code was loaded from a file its likely to be running for a while so we'll long
+        // live it and collect any garbage before running.
+        if (input_kind == MP_PARSE_FILE_INPUT) {
+            module_fun = make_obj_long_lived(module_fun, 6);
+            gc_collect();
+        }
+
         // execute code
         mp_hal_set_interrupt_char(CHAR_CTRL_C); // allow ctrl-C to interrupt us
         start = mp_hal_ticks_ms();
diff --git a/main.c b/main.c
@@ -44,6 +44,7 @@
 #include "lib/utils/pyexec.h"
 
 #include "mpconfigboard.h"
+#include "supervisor/cpu.h"
 #include "supervisor/port.h"
 #include "supervisor/filesystem.h"
 // TODO(tannewt): Figure out how to choose language at compile time.
@@ -381,16 +382,17 @@ int __attribute__((used)) main(void) {
 }
 
 void gc_collect(void) {
-    // WARNING: This gc_collect implementation doesn't try to get root
-    // pointers from CPU registers, and thus may function incorrectly.
-    void *dummy;
     gc_collect_start();
+
+    mp_uint_t regs[10];
+    mp_uint_t sp = cpu_get_regs_and_sp(regs);
+
     // This collects root pointers from the VFS mount table. Some of them may
     // have lost their references in the VM even though they are mounted.
     gc_collect_root((void**)&MP_STATE_VM(vfs_mount_table), sizeof(mp_vfs_mount_t) / sizeof(mp_uint_t));
     // This naively collects all object references from an approximate stack
     // range.
-    gc_collect_root(&dummy, ((mp_uint_t)&_estack - (mp_uint_t)&dummy) / sizeof(mp_uint_t));
+    gc_collect_root((void**)sp, ((uint32_t)&_estack - sp) / sizeof(uint32_t));
     gc_collect_end();
 }
 
diff --git a/ports/atmel-samd/Makefile b/ports/atmel-samd/Makefile
@@ -412,13 +412,16 @@ endif
 SRC_SHARED_MODULE_EXPANDED = $(addprefix shared-bindings/, $(SRC_SHARED_MODULE)) \
                              $(addprefix shared-module/, $(SRC_SHARED_MODULE))
 
+SRC_S = supervisor/$(CHIP_FAMILY)_cpu.s
+
 OBJ = $(PY_O) $(SUPERVISOR_O) $(addprefix $(BUILD)/, $(SRC_C:.c=.o))
 OBJ += $(addprefix $(BUILD)/, $(SRC_ASF:.c=.o))
 OBJ += $(addprefix $(BUILD)/, $(SRC_COMMON_HAL_EXPANDED:.c=.o))
 OBJ += $(addprefix $(BUILD)/, $(SRC_SHARED_MODULE_EXPANDED:.c=.o))
 ifeq ($(INTERNAL_LIBM),1)
 OBJ += $(addprefix $(BUILD)/, $(SRC_LIBM:.c=.o))
 endif
+OBJ += $(addprefix $(BUILD)/, $(SRC_S:.s=.o))
 
 SRC_QSTR += $(SRC_C) $(SRC_SUPERVISOR) $(SRC_COMMON_HAL_EXPANDED) $(SRC_SHARED_MODULE_EXPANDED) $(STM_SRC_C)
 
diff --git a/ports/atmel-samd/supervisor/samd21_cpu.s b/ports/atmel-samd/supervisor/samd21_cpu.s
@@ -0,0 +1,35 @@
+.syntax unified
+.cpu cortex-m0
+.thumb
+.text
+.align  2
+
+@ uint cpu_get_regs_and_sp(r0=uint regs[10])
+.global cpu_get_regs_and_sp
+.thumb
+.thumb_func
+.type   cpu_get_regs_and_sp, %function
+cpu_get_regs_and_sp:
+@ store registers into given array
+str     r4, [r0, #0]
+str     r5, [r0, #4]
+str     r6, [r0, #8]
+str     r7, [r0, #12]
+push {r1}
+mov     r1, r8
+str     r1, [r0, #16]
+mov     r1, r9
+str     r1, [r0, #20]
+mov     r1, r10
+str     r1, [r0, #24]
+mov     r1, r11
+str     r1, [r0, #28]
+mov     r1, r12
+str     r1, [r0, #32]
+mov     r1, r13
+str     r1, [r0, #36]
+pop {r1}
+
+@ return the sp
+mov     r0, sp
+bx      lr
diff --git a/ports/atmel-samd/supervisor/samd51_cpu.s b/ports/atmel-samd/supervisor/samd51_cpu.s
@@ -0,0 +1,27 @@
+.syntax unified
+.cpu cortex-m4
+.thumb
+.text
+.align  2
+
+@ uint cpu_get_regs_and_sp(r0=uint regs[10])
+.global cpu_get_regs_and_sp
+.thumb
+.thumb_func
+.type   cpu_get_regs_and_sp, %function
+cpu_get_regs_and_sp:
+@ store registers into given array
+str     r4, [r0], #4
+str     r5, [r0], #4
+str     r6, [r0], #4
+str     r7, [r0], #4
+str     r8, [r0], #4
+str     r9, [r0], #4
+str     r10, [r0], #4
+str     r11, [r0], #4
+str     r12, [r0], #4
+str     r13, [r0], #4
+
+@ return the sp
+mov     r0, sp
+bx      lr
diff --git a/ports/nrf/Makefile b/ports/nrf/Makefile
@@ -208,6 +208,8 @@ SRC_SHARED_BINDINGS = \
 SRC_SHARED_MODULE_EXPANDED = $(addprefix shared-bindings/, $(SRC_SHARED_BINDINGS)) \
                              $(addprefix shared-module/, $(SRC_SHARED_MODULE))
 
+SRC_S = supervisor/cpu.s
+
 FROZEN_MPY_PY_FILES := $(shell find -L $(FROZEN_MPY_DIR) -type f -name '*.py')
 FROZEN_MPY_MPY_FILES := $(addprefix $(BUILD)/,$(FROZEN_MPY_PY_FILES:.py=.mpy))
 
@@ -218,6 +220,7 @@ OBJ += $(addprefix $(BUILD)/, $(SRC_NRFX:.c=.o))
 OBJ += $(addprefix $(BUILD)/, $(DRIVERS_SRC_C:.c=.o))
 OBJ += $(addprefix $(BUILD)/, $(SRC_COMMON_HAL_EXPANDED:.c=.o))
 OBJ += $(addprefix $(BUILD)/, $(SRC_SHARED_MODULE_EXPANDED:.c=.o))
+OBJ += $(addprefix $(BUILD)/, $(SRC_S:.s=.o))
 
 $(BUILD)/$(FATFS_DIR)/ff.o: COPT += -Os
 $(filter $(PY_BUILD)/../extmod/vfs_fat_%.o, $(PY_O)): COPT += -Os
diff --git a/ports/nrf/supervisor/cpu.s b/ports/nrf/supervisor/cpu.s
@@ -0,0 +1,27 @@
+.syntax unified
+.cpu cortex-m4
+.thumb
+.text
+.align  2
+
+@ uint cpu_get_regs_and_sp(r0=uint regs[10])
+.global cpu_get_regs_and_sp
+.thumb
+.thumb_func
+.type   cpu_get_regs_and_sp, %function
+cpu_get_regs_and_sp:
+@ store registers into given array
+str     r4, [r0], #4
+str     r5, [r0], #4
+str     r6, [r0], #4
+str     r7, [r0], #4
+str     r8, [r0], #4
+str     r9, [r0], #4
+str     r10, [r0], #4
+str     r11, [r0], #4
+str     r12, [r0], #4
+str     r13, [r0], #4
+
+@ return the sp
+mov     r0, sp
+bx      lr
diff --git a/py/builtinimport.c b/py/builtinimport.c
@@ -31,6 +31,7 @@
 
 #include "py/compile.h"
 #include "py/gc_long_lived.h"
+#include "py/gc.h"
 #include "py/objmodule.h"
 #include "py/persistentcode.h"
 #include "py/runtime.h"
@@ -468,6 +469,10 @@ mp_obj_t mp_builtin___import__(size_t n_args, const mp_obj_t *args) {
                     // (the module that was just loaded) is not a package.  This will be caught
                     // on the next iteration because the file will not exist.
                 }
+
+                // Loading a module thrashes the heap significantly so we explicitly clean up
+                // afterwards.
+                gc_collect();
             }
             if (outer_module_obj != MP_OBJ_NULL) {
                 qstr s = qstr_from_strn(mod_str + last, i - last);
diff --git a/py/gc.c b/py/gc.c
diff --git a/py/gc_long_lived.c b/py/gc_long_lived.c
@@ -121,7 +121,7 @@ mp_obj_t make_obj_long_lived(mp_obj_t obj, uint8_t max_depth){
     } else if (MP_OBJ_IS_TYPE(obj, &mp_type_property)) {
         mp_obj_property_t *prop = MP_OBJ_TO_PTR(obj);
         return MP_OBJ_FROM_PTR(make_property_long_lived(prop, max_depth));
-    } else if (MP_OBJ_IS_TYPE(obj, &mp_type_str)) {
+    } else if (MP_OBJ_IS_TYPE(obj, &mp_type_str) || MP_OBJ_IS_TYPE(obj, &mp_type_bytes)) {
         mp_obj_str_t *str = MP_OBJ_TO_PTR(obj);
         return MP_OBJ_FROM_PTR(make_str_long_lived(str));
     } else if (MP_OBJ_IS_TYPE(obj, &mp_type_type)) {
diff --git a/py/mpconfig.h b/py/mpconfig.h
@@ -130,6 +130,12 @@
 #define MICROPY_ALLOC_QSTR_CHUNK_INIT (128)
 #endif
 
+// Max number of entries in newly allocated QSTR pools. Smaller numbers may make QSTR lookups
+// slightly slower but reduce the waste of unused spots.
+#ifndef MICROPY_QSTR_POOL_MAX_ENTRIES
+#define MICROPY_QSTR_POOL_MAX_ENTRIES (64)
+#endif
+
 // Initial amount for lexer indentation level
 #ifndef MICROPY_ALLOC_LEXER_INDENT_INIT
 #define MICROPY_ALLOC_LEXER_INDENT_INIT (10)
diff --git a/py/qstr.c b/py/qstr.c
@@ -144,14 +144,18 @@ STATIC qstr qstr_add(const byte *q_ptr) {
 
     // make sure we have room in the pool for a new qstr
     if (MP_STATE_VM(last_pool)->len >= MP_STATE_VM(last_pool)->alloc) {
-        qstr_pool_t *pool = m_new_ll_obj_var_maybe(qstr_pool_t, const char*, MP_STATE_VM(last_pool)->alloc * 2);
+        uint32_t new_pool_length = MP_STATE_VM(last_pool)->alloc * 2;
+        if (new_pool_length > MICROPY_QSTR_POOL_MAX_ENTRIES) {
+            new_pool_length = MICROPY_QSTR_POOL_MAX_ENTRIES;
+        }
+        qstr_pool_t *pool = m_new_ll_obj_var_maybe(qstr_pool_t, const char*, new_pool_length);
         if (pool == NULL) {
             QSTR_EXIT();
-            m_malloc_fail(MP_STATE_VM(last_pool)->alloc * 2);
+            m_malloc_fail(new_pool_length);
         }
         pool->prev = MP_STATE_VM(last_pool);
         pool->total_prev_len = MP_STATE_VM(last_pool)->total_prev_len + MP_STATE_VM(last_pool)->len;
-        pool->alloc = MP_STATE_VM(last_pool)->alloc * 2;
+        pool->alloc = new_pool_length;
         pool->len = 0;
         MP_STATE_VM(last_pool) = pool;
         DEBUG_printf("QSTR: allocate new pool of size %d\n", MP_STATE_VM(last_pool)->alloc);
diff --git a/supervisor/cpu.h b/supervisor/cpu.h
@@ -0,0 +1,34 @@
+/*
+ * This file is part of the MicroPython project, http://micropython.org/
+ *
+ * The MIT License (MIT)
+ *
+ * Copyright (c) 2017 Scott Shawcroft for Adafruit Industries
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef MICROPY_INCLUDED_SUPERVISOR_CPU_H
+#define MICROPY_INCLUDED_SUPERVISOR_CPU_H
+
+// Adds up to 10 pointers from the CPUs registers to regs. This is used to make sure no actively
+// used heap memory is freed. Its usually implemented in assembly.
+mp_uint_t cpu_get_regs_and_sp(mp_uint_t *regs);
+
+#endif  // MICROPY_INCLUDED_SUPERVISOR_CPU_H
diff --git a/tools/analyze_heap_dump.py b/tools/analyze_heap_dump.py
diff --git a/tools/output_gc_until_repl.txt b/tools/output_gc_until_repl.txt

Original file line number	Diff line number	Diff line change
`@@ -58,3 +58,4 @@ TAGS`
`58`	`58`	`#################`
`59`	`59`	`*.orig`
`60`	`60`
	`61`	`+*.DS_Store`