Processing.js, Javascript object usage leaks memory!

[lgto4]

Refer to the simple demo program at https://www.khanacademy.org/computer-programming/leak/5149570618228736 which simply creates and ignores new objects in the draw function. The program's memory usage grows without bounds until the browser page fails.

I first noticed the program on a PVector intensive program. Other users have also complained, including one that observed the tutorial on Partical Systems at https://www.khanacademy.org/computing/computer-programming/programming-natural-simulations/programming-particle-systems/a/a-particle-system also fails.

The Comments to the Help Request of the test program also identifies where in the KA code that the leak is occurring.

[GoToLoop]

JS, like most modern languages, relies on a garbage collector in order to get rid of any objects which aren't being referenced by any variables or properties anymore. However, JS' GC isn't as aggressive as for example Java's. If we keep creating objects like there's no tomorrow, the GC won't keep up destroying objects as fast as new 1s are made! I think it's more JS language related than Khan's pjs particular fault...

[kevinbarabash]

I think there might be some things we can do to mitigate this. We do track instances so we can recreate them if necessary and inject them when a person makes a change to the code. We could do something about new-ing up an object without storing it in a variable (the linter should probably catch this). I'm not sure what to do about other objects. The reason the GC isn't cleaning them up is because live-editor still has a reference to them.

[lgto4]

@GoToLoop: That is a fine thesis, but can it stand scurtiny?

1. Is Javascript the computer language equivalent of the cheap toy truck that I got for Christmas whose wheels fell off late Christmas evening just because I pushed it around the tree five hundred times? Should it come with a disclaimer: Warning: do not use Javascript for serious computing. Warning: Javascript is known to cause crashes in unattended or long-lived web pages."
2. When the same test program is run on a private page or on a "more standard" server like JsFiddle.net (e.g. http://jsfiddle.net/blyon/2xbbwy2o/), the memory requirements are constant for as long as one wishes to observe. (That constant is about two thirds of the initial KA memory requirement.)
3. The bug report mentions the pointer to the suspected KA code that leaks. (I am not competent to assess the code.)

[lgto4]

@kevinbarabash: I don't need the simple test case mitigated. It is a result of narrowing down a legitimate PVector intensive program (https://www.khanacademy.org/computer-programming/p/6381256892350464) that could not stay up for more than half an hour.

Is this another example of "forward" progress justifying breakage? The author of this one year old program https://www.khanacademy.org/computer-programming/winterful-level-pack-6-levels-5-big-levels/4843946103537664 just posted a help request: "(I know nobody will look at this but whatever) It would be nice to know why the frame rate drops until it freezes. Didn't do that until recently, and i haven't touched the code (from when I first changed it)."

Should Pamela update the “ADVANCED JS: NATURAL SIMULATIONS” tutorials with a warning that Partical Systems are bound to fail (including those embedded in the tutorials)?

On the bright side, this can give noobs an introduction to the workings of memory management systems in a similar manner that my daughter was introduced to the workings of automobile transmissions.

[ghost]

In all honesty, after a certain level of programming, it is just too impractical to have the live refresh. It always leads to problems. We still have core and user functions being converted to strings when objects are changed.

Play around with this... I don't get it. It's becoming hard to reproduce the bug, but I always manage to.

There's also the spin-off with the infamous color bug. For even more problems, change the code of the ultimate tower defense 2 at a random mid-wave moment and see what happens. It's probably not good. I just tried and it restarted the game on Safari 9.0.1 and I don't know about other browsers.

[kevinbarabash]

@DozenalFTW converting functions to strings causes problems for us. I think that we could probably change how we're doing this now that we're turning all globals into member expressions. It might also be good to have an option for users to turn off live updating.

What's the infamous colour bug? Also, I tried changing values around the scratchpad that you linked to but I didn't see any errors. What's supposed to happen?

[ghost]

1. Weird, there was a bug where if you had objects in your program and changed certain properties, you'd get errors about strings not being functions. Well, this seems to be fixed! :+1:
2. There used to be a bug where objects would cause all colors to go black. This has mutated: change the value on line 5, or the colors themselves, and both rects disappear.
   • Something to note is that changing the rect() call on line 9 doesn't update anything at all. I think at this level you're just going to have to press Restart... it would be of no value to inject even MORE slow code to handle this.

[kevinbarabash]

@DozenalFTW thanks for the example. I've had a look at the code and determined that we were stringifying ProcessingJS functions like fill, noStroke, etc. and then replacing the functions with the strings, whoops. I'm surprised that our tests didn't pick this up. I was able to fix that issue, but changing object.method doesn't update like it should.

[kevinbarabash]

@lgto4 sorry for the delayed response. I had a look at the PVector heavy program you linked. It's troublesome that the program ran out of memory even after you went to the trouble of recycling object instances. A person shouldn't have to do that when the instances are being used temporarily. We should be able to rely on the garbage collector.

In the case of the snowman game I wasn't able to tell if the particles were being removed from the scene or not. In those cases where the user adds a whole bunch of objects and always has a reference to them we should give some kind of warning to the user and provide some guidance in how to deal with such issues.

That being said, it sounds like the current issues with the snowman game are a regression. I'm going to compare memory consumption of the current live-editor against an older version from before we started using AST transforms and see where the increase in memory use is coming from. One possible reason for the increase in memory is that we're calling .toString() on methods after code is transformed which means there's additional code in each method which makes the .toString() methods longer. I also did some refactoring in an attempt to make .injectCode() easier to understand. It's possible that some of those changes have made it so that we're storing more copies of objects than we should.

Is this another example of "forward" progress justifying breakage?

We'd like to avoid breakages when making progress. We do have lots of tests, but we could be doing more in the testing area. I think some low-hanging fruit would be around automating challenge regression tests. That doesn't really help with memory/performance issues.

I would eventually like to change how we do code injection to remove the need for calling .toString() on everything. I have a hunch on how to do this but not a clear picture yet.

[kevinbarabash]

I found the memory leak and fixed it. The changes should be deployed by EOW.

[kevinbarabash]

Memory leak should be fixed now. Let me know if it's not.

[prolightHub]

It's definitely not fixed.