Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quickly running out of memory in while loop #50

Closed
maxint-rd opened this issue Feb 4, 2022 · 7 comments
Closed

Quickly running out of memory in while loop #50

maxint-rd opened this issue Feb 4, 2022 · 7 comments

Comments

@maxint-rd
Copy link

maxint-rd commented Feb 4, 2022

Hello and thank you for sharing your code. It's a wonderful lightweight scripting engine that's easy to use while providing powerful possibilities.

While trying to use it for my ESP8266 device, I unfortunately ran into some memory issues.
This was the simple script I tried to run:
let n=10; while(n-->0) { console.log("u"+str(n)+":"+str(usage())+"\n"); }
The console.log function is a simple FFI printf addition I used to get debug output and usage calls your js_usage to report the percentage of memory used.

When running this script I quickly ran into an OOM error. So first I increased my js-buffer, but that wasn't enough to allow a much greater amount of iterations (say n=100). I noticed that each iteration would substantially increase the amount of memory used. At the global scope the memory would be released by the garbage collection, but not within a block.
After three days of debugging, I learned more of your code and I found a solution that worked for me.

After adding some debug statements it appeared that the Fast scope GC line within js_block() never really released any memory:

printf("js_block[s%u] pre GC %u (1:%u, 2:%u) \n", (jsoff_t) vdata(js->scope), js->brk, brk1, brk2);
if (js->brk == brk2) js->brk = brk1;  // Fast scope GC (ORIGINAL CODE)
printf("js_block[s%u] post GC %u-%u=%u\n", (jsoff_t) vdata(js->scope), js->brk, brk1, js->brk-brk1);

I saw that js->brk would always be different from brk2. Skipping that check to always call js->brk = brk1 did set the memory-scope back, but resulted in corruption for instance with string variables that had text added within the loop.

What finally worked for me was to call the slower js_gc() function instead:

printf("js_block[s%u] pre GC %u (1:%u, 2:%u) \n", (jsoff_t) vdata(js->scope), js->brk, brk1, brk2);
if((js->flags & F_NOEXEC) != F_NOEXEC)   // only do gc() when executing (i.e not when defining function in object)
  js_gc(js);   // Slower scope GC (NEW CODE)
printf("js_block[s%u] post GC %u-%u=%u\n", (jsoff_t) vdata(js->scope), js->brk, brk1, js->brk-brk1);

This modification allowed me to run much larger iterations without loosing any memory.

From the looks of it, this issue was introduced in one of the may-2021 releases where a call to js_gc() within js_block() was removed, but I guess you have a better environment to debug and improve your code. I just wanted to share my experience and give people running into the same issue a possible solution (providing no guarantee that what worked for me will also work for you).

Edit: had some issues with objects containing functions. Fixed by checking for F_NOEXEC

@cpq
Copy link
Member

cpq commented Feb 6, 2022

@maxint-rd thanks. Could you clarify, what version you're using please.

@maxint-rd
Copy link
Author

maxint-rd commented Feb 6, 2022

Hello, I've downloaded the most recent version from GitHub, one week ago (29-jan-2022), so that would be version Elk 2.2.0. Note that I downloaded the master tree directly ( not the release under https://github.com/cesanta/elk/releases/tag/2.2.0).
Since I'm still developing the application for my ESP, I didn't have time to make an separate installation to test the core library and make you an isolated test case,

@cpq
Copy link
Member

cpq commented Feb 6, 2022

@maxint-rd thank you. I assume you're making your loop inside a function. A loop in the root scope should GC fine.
Looking into it.

@cpq
Copy link
Member

cpq commented Feb 7, 2022

@maxint-rd pushed 844fafe which should fix an issue. What is the problem with objects - could you share a JS snippet what demonstrates an issue please?

@maxint-rd
Copy link
Author

Thank you for your swift replies. When I get the time later this week I will give it a try and supply you with snippets that demonstrate any remaining issues. Thank you for your support.

@maxint-rd
Copy link
Author

maxint-rd commented Feb 12, 2022

Hello and thank you for this update. I tried it out in my application and it seems to make some significant changes. In my application I have implemented JS processing of serial data where I can just enter a line of Javascript that will then be evaluated (with persistent global JS memory to allow reuse of functions and variables).
I didn't have a lot of time to test everything, but can confirm that for the while loop garbage collection works fine now. Great!
However, I still encountered some issues in some of my tests. They seem to be similar to what I first had with defining functions in an object. Which I solved by checking for the F_NOEXEC flag.

A bit more detail... In this test something went wrong, indicative of memory corruption (perhaps garbage collected of data still in use):
let yyy = {a:123, f:function(x) { return x * 3; }}; yyy.f(3);
The expected result is 9, but instead my application hangs and the watchdog kicks in. Just doing this also goes wrong:
let zzz = {a:123, f:function(x) { return x * 3; }};
Defining an object without a function and returning a member doesn't go wrong:
let xxx = {a:123, g:321}; xxx.a;
I think the issue has to do with processing the function block and storing it with the object. I suspect that garbage is collected on it and memory is freed at definition time, something that also went wrong in the previous version (before my mod).

I continued and did some debugging/fixing. I managed to resolve the issue the same way as before, i.e. only do GC when F_NOEXEC is not set. So in top of js_stmt() I replace the call to js_gc() by this:

if((js->flags & F_NOEXEC) != F_NOEXEC)   // only do gc() when executing (i.e not when defining function in object)
    js_gc(js);

I still have some things remaining, but these are probably not related to GC and as soon as I can find some time I will try to isolate them and post them as separate issues. Thank you for your work and support!

@maxint-rd
Copy link
Author

maxint-rd commented Feb 13, 2022

Hello, as I still had problems, today I spent a long day debugging...
This is a minimal version of a longer script that went wrong. I tested it numerous times to debug the code. You can try it to see if you can reproduce my experience:
let i=0; let res=""; while(i<10) { res=res+"!"; i++; }
The script above caused memory corruption, resulting in random behavior of my ESP8266, varying from watch-dog resets, to the JS engine failing to work and others issues.

When testing these slight modifications of the script above, the behavior would be different or even not be problematic:

let i=0; let res=""; while(i<1) { res=res+"!"; i++; }
let i=0; let res=""; while(i++<10) { res=res+"!"; }
let i=0; let res="abc"; while(i<10) { res=res+"!"; i++; }

By inserting gazillion debug printf's I managed to identify the cause of the corruption: the TOK_PLUS operation in do_string_op() would do a memmove of invalid length (-1 instead of 0).
I think I also found the possible root cause of this issue and it brought me back to previous experiences. In js_block you still had this code:

 if (js->brk == brk2) js->brk = brk1;  // Fast scope GC   <== THIS CAUSES PROBLEMS!

Since that code is no longer needed due to the js_gc() call in js_stmt(), and it never really worked properly (eg. issues in function definitions), I commented it out. I'm still testing if that didn't break other things, but for this test case the modification helped.

Hope you can understand my happiness and that you can have a look at that particular line of code.

@cpq cpq closed this as completed in 08906bd Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants