espruino / Espruino

The Espruino JavaScript interpreter - Official Repo
http://www.espruino.com/
Other
2.75k stars 739 forks source link

Unfreed block #2345

Closed notEvil closed 1 year ago

notEvil commented 1 year ago

Hi,

I was investigating a slow memory usage buildup when I saw a block appearing that didn't make sense. To reproduce, compile 2v17 for Linux with

```diff diff --git a/Makefile b/Makefile index 203404f2c..c03b8fecb 100755 --- a/Makefile +++ b/Makefile @@ -230,7 +230,7 @@ endif ifdef DEBUG #OPTIMIZEFLAGS=-Os -g ifeq ($(FAMILY),ESP8266) - OPTIMIZEFLAGS=-g -Os -std=gnu11 -fgnu89-inline -Wl,--allow-multiple-definition + OPTIMIZEFLAGS=-g -Os -std=gnu17 -fgnu89-inline -Wl,--allow-multiple-definition else OPTIMIZEFLAGS=-g endif diff --git a/make/misc/tensorflow.make b/make/misc/tensorflow.make index 7af60dc9c..9c575e897 100644 --- a/make/misc/tensorflow.make +++ b/make/misc/tensorflow.make @@ -73,5 +73,5 @@ ifdef RELEASE CCFLAGS += -DNDEBUG # to strip all error messages add -DTF_LITE_STRIP_ERROR_STRINGS endif -CCFLAGS += -fmacro-prefix-map=$(TENSOR_ROOT)/= -g -DTF_LITE_STATIC_MEMORY --std=c++11 -g -fno-rtti -fpermissive -Wno-sign-compare -Wno-conversion -Wno-sign-conversion -Wno-missing-field-initializers -Wno-type-limits -Wno-unused-parameter -Wno-unused-variable +CCFLAGS += -fmacro-prefix-map=$(TENSOR_ROOT)/= -g -DTF_LITE_STATIC_MEMORY --std=c++17 -g -fno-rtti -fpermissive -Wno-sign-compare -Wno-conversion -Wno-sign-conversion -Wno-missing-field-initializers -Wno-type-limits -Wno-unused-parameter -Wno-unused-variable DEFINES += -DUSE_TENSORFLOW=1 diff --git a/src/jsvar.c b/src/jsvar.c index 2c7a8c009..260ec76b1 100644 --- a/src/jsvar.c +++ b/src/jsvar.c @@ -4131,7 +4131,7 @@ void jsvDefragment() { // Dump any locked variables that aren't referenced from `global` - for debugging memory leaks void jsvDumpLockedVars() { - jsvGarbageCollect(); + // jsvGarbageCollect(); if (isMemoryBusy) return; isMemoryBusy = MEMBUSY_SYSTEM; JsVarRef i; ```

(had to use C++17 because C++11 showed deprecation errors for abseil), then execute

>E.dumpLockedVars()
#4[r0,l1] Array(0) [ ]
#5[r0,l2] String [1 blocks] "E.dumpLockedVars()" 
#17[r0,l1] String [1 blocks] "" 
#20[r1,l1] NativeFunction 0x7c1ee35d (0) { }
#21[r0,l1] NewChild PARENT:  #18[r4,l2] NativeFunction 0x7c18680c (1) { }CHILD: String [2 blocks] "dumpLockedVars"  #20[r1,l1] NativeFunction 0x7c1ee35d (0) { } 
=undefined

>E.dumpVariables()
ref,size,flags,name,links...
1,1,1,Object,2,11,13,19,
2,1,28,"\xFF",3,
3,1,5,Object,6,8,16,20,
4,1,3,Array,
5,1,54,String "E.dumpLockedVars()",
6,1,33,"timers",7,
7,1,3,Array,
8,1,34,"watches",9,
9,1,3,Array,
10,1,8,,
11,1,31,"quit",10,
12,1,8,,
13,2,35,"interrupt",12,
15,1,48,String "\0\0\0\0\0\xFF\xFF\xFF\0\0\0\0",
16,1,30,"net",15,
17,1,36,String "",
18,1,8,,
19,1,28,"E",18,
20,1,34,"history",21,
21,1,3,Array,22,
22,1,15,"0",5,
23,1,53,String "E.dumpVariables()",
24,1,8,,
25,2,35,"dumpVariables",24,
=undefined

>(() => 1)
=function () { ... }

>E.dumpLockedVars()
#4[r0,l1] Array(0) [ ]
#17[r0,l2] String [1 blocks] "E.dumpLockedVars()" 
#26[r0,l1] String [1 blocks] "" 
#27[r0,l1] NewChild PARENT:  #18[r4,l2] NativeFunction 0x7c18680c (1) { }CHILD: String [2 blocks] "dumpLockedVars"  #28[r1,l1] NativeFunction 0x7c1ee35d (0) { } 
=undefined

>E.dumpVariables()
ref,size,flags,name,links...
1,1,1,Object,2,11,13,19,
2,1,28,"\xFF",3,
3,1,5,Object,6,8,16,20,
4,1,3,Array,
5,1,36,String "",
6,1,33,"timers",7,
7,1,3,Array,
8,1,34,"watches",9,
9,1,3,Array,
10,1,8,,
11,1,31,"quit",10,
12,1,8,,
13,2,35,"interrupt",12,
15,1,48,String "\0\0\0\0\0\xFF\xFF\xFF\0\0\0\0",
16,1,30,"net",15,
17,1,54,String "E.dumpLockedVars()",
18,1,8,,
19,1,28,"E",18,
20,1,34,"history",21,
21,1,3,Array,24,29,22,
22,1,15,"3",17,
23,1,53,String "E.dumpVariables()",
24,1,15,"1",23,
25,1,45,String "(() => 1)",
26,1,53,String "E.dumpVariables()",
27,2,35,"dumpVariables",28,
28,1,8,,
29,1,15,"2",25,
=undefined

>(() => 1)
=function () { ... }

>E.dumpLockedVars()
#4[r0,l1] Array(0) [ ]
#23[r0,l2] String [1 blocks] "E.dumpLockedVars()" 
#25[r0,l0] Unknown 66 
#27[r0,l1] NewChild PARENT:  #18[r4,l2] NativeFunction 0x7c18680c (1) { }CHILD: String [2 blocks] "dumpLockedVars"  #30[r1,l1] NativeFunction 0x7c1ee35d (0) { } 
#28[r0,l1] String [1 blocks] "" 
=undefined

>E.dumpVariables()
ref,size,flags,name,links...
1,1,1,Object,2,11,13,19,
2,1,28,"\xFF",3,
3,1,5,Object,6,8,16,20,
4,1,3,Array,
5,1,45,String "(() => 1)",
6,1,33,"timers",7,
7,1,3,Array,
8,1,34,"watches",9,
<b>9,1,3,Array,</b>
10,1,8,,
11,1,31,"quit",10,
12,1,8,,
13,2,35,"interrupt",12,
15,1,48,String "\0\0\0\0\0\xFF\xFF\xFF\0\0\0\0",
16,1,30,"net",15,
17,1,36,String "",
18,1,8,,
19,1,28,"E",18,
20,1,34,"history",21,
21,1,3,Array,24,29,22,
22,1,15,"6",23,
23,1,54,String "E.dumpLockedVars()",
24,1,15,"4",26,
26,1,53,String "E.dumpVariables()",
27,2,35,"dumpVariables",30,
28,1,53,String "E.dumpVariables()",
29,1,15,"5",5,
30,1,8,,
=undefined

Block 25 with flags 66 shouldn't be there. Using a function definition is significant. The block magically disappears when flooding the command history with different commands. Hope this description helps identifying the relevant code sections. I tried but didn't succeed.

notEvil commented 1 year ago

I think I figured it out: its the StringExt to the name "dumpLockedVars" (block 27). So there is no issue, except maybe suboptimal output. It would probably require an additional pass to identify solitary StringExt.

gfwilliams commented 1 year ago

Ok, great. Well, that makes sense I guess.

I notice you commented out jsvGarbageCollect in DumpLockedVars - did it not appear if that was left in?

Presumably it's just bad luck that in this case the StringExt came before the String that referenced it, when usually it would appear after?

notEvil commented 1 year ago

I notice you commented out jsvGarbageCollect in DumpLockedVars - did it not appear if that was left in?

It does appear because it is part of the current function call and therefore locked.

Presumably it's just bad luck that in this case the StringExt came before the String that referenced it, when usually it would appear after?

Since the free list isn't guaranteed to always link forwards, it is bad luck, sort of. I have no numbers, but I'd say it is rather common.