bugGnash - The GNU Flash player - Bugs: bug #28009, Potlatch photos make gnash eat all...

 
 

bug #28009: Potlatch photos make gnash eat all memory

Submitter:  Sandro Santilli <strk>
Submitted:  Thu 12 Nov 2009 10:30:02 PM UTC
Votes: 50
 
Category:  core Severity:  4 - Important
Release:  None Status:  None
Privacy:  Public Assigned to:  None
Open/Closed:  Open
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 24 May 2012 12:15:25 PM UTC, comment #25: 

Just a pointer about how to trigger GC run: set_new_handler can be called (standard C++ interface) to install a function that would be called on out-of-memory condition. The function would run the GC and give 'new' another chance of succeeding:

http://stackoverflow.com/questions/5548639/what-can-new-handler-be-used-for-in-c-besides-garbage-collection

Such behavior won't fix memory fragmentation, but it is something to look at.

Sandro Santilli <strk>
Group Member
Tue 13 Jul 2010 02:57:08 PM UTC, comment #24: 

Not on the Gnash side (there are plans but no funding for that).

On the potlatch side I've filed a patch [1] but no action was
taken for it, consider pinging there.

[1] http://trac.openstreetmap.org/ticket/2929

Sandro Santilli <strk>
Group Member
Tue 13 Jul 2010 02:53:51 PM UTC, comment #23: 

Hi there, any news on this issue?

Mind Booster Noori <mindbooster>
Tue 13 Jul 2010 02:51:53 PM UTC, comment #22: 

Hi there, any news on this issue?

Mind Booster Noori <mindbooster>
  Spam posted by annaanna1
  Spam posted by lindatan
  Spam posted by anonymous
Mon 14 Dec 2009 09:40:02 PM UTC, comment #18: 

Anoher statistic (since I'm at it):
by setting GNASH_GC_TRIGGER_THRESHOLD=1 (that means, a single CollectableResource allocated is enough for the GC to run and cleanup) the memory use for innocent6 testcase is 260m.

So to recap:

1: Only collecting at the end of AS execution: 723m
2: Collecting every 50 allocated objects     : 324m
3: Collecting every allocated object         : 260m

Interesting figures for a plan...

Sandro Santilli <strk>
Group Member
Mon 14 Dec 2009 09:33:11 PM UTC, comment #17: 

I attach the patch which applied to r11694 runs the GC at every branch-back opcode.

It's not meant to be committed as there are surely better times
to run the GC than on branch-back, but is a proof of concept
about how memory use is reduced by releasing memory earlier.

Still not sure why the late-released-memory still sticks around
even with GLIBCXX_FORCE_NEW defined, anyway on low-memory devices
its still good to reduce memory usage (even out of loops:)

For the record, innocent6.as runs with 324m with early GC
vs. 723m with late GC. Around 400mb of memory saved....

(file #19267)

Sandro Santilli <strk>
Group Member
Sun 13 Dec 2009 09:10:56 PM UTC, comment #16: 

So, a quick test running the GC collector at every branch-back
from ActionExec shows that the problem would be fixed by that.

Now, if we want to go there we must make sure resources
on the stack and CallStack and all registers (global and function local) are marked too as we can't ensure anymore they are empty.

I committed a patch to as_environment::markReachableResources
to mark the CallStack and _stack, but that's not the correct
place to do so as those members of as_environment are simply
aliases into the VM ones, so rather then marking them again
for each environment (one each movieclip) we should mark them
once from within VM.

Anyway, the test shows that clearing resources earlier (trouh GC)
makes a big difference in memory usage.
Not sure why, maybe memory fragmentation has a role in this ?

Sandro Santilli <strk>
Group Member
Sun 13 Dec 2009 05:29:38 PM UTC, comment #15: 

Alright, even just doing o = new Object inside that loop
spins memory up.

I'm afraid the only way to reduce this would be running the
GC more often. Maybe doing it at every branch or so would help.
Just we'll need to mark also the stack values and CallStack
if we go there.

The threshold shouldn't make this too much expensive

Sandro Santilli <strk>
Group Member
Sun 13 Dec 2009 02:34:42 PM UTC, comment #14: 

Final simplification. Single loop.

(file #19250)

Sandro Santilli <strk>
Group Member
Sun 13 Dec 2009 02:18:32 PM UTC, comment #13: 

Another simplification, less variables, less opcodes

(file #19249)

Sandro Santilli <strk>
Group Member
Sun 13 Dec 2009 01:16:12 PM UTC, comment #12: 

More simplification:
 - No confusing 'var' (there are no locals w/out functions anyway)
 - Marametrize number of iterations in inner and outer loops
 - Reduce the size of string 'substr' is called against
 - Drop the conditional, always call substr

Test still show the pp memory usage is stable while changing
number of inner iterations and gnash isn't. Details in the sourcecode.

(file #19248)

Sandro Santilli <strk>
Group Member
Sun 13 Dec 2009 12:42:45 PM UTC, comment #11: 

Clearing (deeply, with release of any allocations) SafeStack after each advanceMovie doesn't fix so the culprit is not SafeStack.

I attach a further simplified test which drops any function.

(file #19247)

Sandro Santilli <strk>
Group Member
Sat 12 Dec 2009 10:56:56 PM UTC, comment #10: 

I suspect the problem here is in SafeStack, which by design
NEVER destroys stack elements (until the stack lives).

Now, since our stack is always alive... any long string
pushed on it will stay forever.


Sandro Santilli <strk>
Group Member
Sat 12 Dec 2009 09:51:13 PM UTC, comment #9: 

If the for loop is substituted by a frame-loop (onEnterFrame)
the memory increas doesn't occur.

Sandro Santilli <strk>
Group Member
Sat 12 Dec 2009 07:59:37 PM UTC, comment #8: 

Simplified testcase, we don't need the second function.
Augmenting the number of iterations increases memory use.
The sourcecode contains information about the magnitude of this.


(file #19243)

Sandro Santilli <strk>
Group Member
Sat 12 Dec 2009 07:55:14 PM UTC, comment #7: 

Finally succeeded in producing a testcase for this.
Attached first version.

Note that a BIG difference is due to the length of the
string passed to the innocent() function.
I've made it around 55 chars which is what potlatch uses (urls)

(file #19242)

Sandro Santilli <strk>
Group Member
Sat 12 Dec 2009 07:41:47 PM UTC, comment #6: 

For the 'memory not returned to system' there's an environment
variable to force flush by stdc++ lib:

export GLIBCPP_FORCE_NEW=1
export GLIBCXX_FORCE_NEW=1

(it changed name depending on gcc version, don't rember when)

Anyway, none of the above reduces the problem
so gnash must be still referencing those objects.

I'm trying to follow the innocent() function but taken
as is doesn't seem to represent a problem.

massif profile would be indeed useful here.

Sandro Santilli <strk>
Group Member
Sat 12 Dec 2009 07:31:04 PM UTC, comment #5: 

I made two changes while also thinking about this, both of which saved a lot of memory under certain circumstances, but neither of those case apply here.

One was to stop constructing as_super automatically, because in function2 calls there is a flag to show when it should be constructed. In built-in class constructors (which Gnash implements in C++ but the pp as function2s), there is also no need to create as_supers before they point they're used (which is not at all in built-in constructors). The as_supers were of course cleaned up at the end of a frame, but during a complex loop they could build up many tens of megabytes.

The other change was to stop making objects for XMLNodes unless they are accessed. Parsing the whole potlatch photos XML file is now fine (swfdec took over 30 minutes before I gave up and killed it) until the nodes are accessed.

The problem with the above two cases seems to be that as_super was never used in the potlatch loop and that all XMLNodes are accessed, so neither thing saves any memory at all for potlatch.

All DisplayObjects that are AS-referenceable have exactly one associated object.

Freed memory may not be returned to the operating system immediately, so that the continuing high usage may not necessarily be a problem. Just the fact it gets so high to start with.

By the way, valgrind's massif tool was really helpful in finding the as_super and XMLNode excesses. I bet a profile of the potlatch case would be interesting, but it is extremely slow.

Benjamin Wolsey <bwy>
Group Member
Sat 12 Dec 2009 07:12:49 PM UTC, comment #4: 

Indeed it's the call to innocent() function raising memory
so much:

(1052m VIRT with just that call)

Function definition:

  // remove non-alphanumerics
  function innocent(a) {
    var b='';
    for (var i=0; i<a.length; i++) {
      var c=a.substr(i,1); if ((c>='A' && c<='Z') || (c>='a' && c<='z') || (c>='0' && c<='9') || c=='_') { b+=c; }
    }
    return b;
  }

Sandro Santilli <strk>
Group Member
Sat 12 Dec 2009 07:09:54 PM UTC, comment #3: 

Another interestings statistic is that if we drop
the whole 'attachMovie' block in photo.as, memory still
grows. Not as much as when attachMovie is called, bust still
way too much (1052m VIRT vs. 1628m).

This is only commenting the block but NOT the condition:

 if (p_icon && !_root.map.photos[innocent(p_icon)]) {
 /*
  var n=innocent(p_icon);
  _root.map.photos.attachMovie("photo",n,++photodepth);
  _root.map.photos[n].init(p_lat,p_lon,p_icon,p_desc,p_name);
 */
 }

If ALSO the condition is commented out, we only get to 510m VIRT.
This means about 1/2 GB allocated and staying resident due
to this single line:

 p_icon && !_root.map.photos[innocent(p_icon)]

!!!

Sandro Santilli <strk>
Group Member
Sat 12 Dec 2009 07:00:31 PM UTC, comment #2: 

Correct, GC doesn't run during an AS loop.

Just tried again with trunk. For the record, gnash reaches
1628m of virtual memory and stays there even after the GC
is run. The number of attached photos is 3417.

CPU usage is also very high once all those things are attached
but that's probably due to the advance calls.

GC resources (from GC tab of movie properties):
12706 as_object
6914 shapes
3531 MovieClip
477 swf_function

My bet is the number of as_object being too high of course.

I wonder, how many new as_object are created per MovieClip/DisplayObject in general ?
We're still using getter/setters for all display object
special properties ?

Sandro Santilli <strk>
Group Member
Fri 13 Nov 2009 10:15:53 AM UTC, comment #1: 

I did some tests with local variables in a loop.

If I'm correct, any new objects created within a loop will not be freed until the GC runs at the end of the advance call.

(loop) {
    var i = new String("hi");
}

One memory hog there is as_super, which is currently created on all constructInstance calls. I don't see any good reason for this at the moment (though I haven't looked that carefully).

Constructing it when it's required in swf_function instead cuts usage from (with 65535 iterations in the loop above) from 128 to 92 MB.

Benjamin Wolsey <bwy>
Group Member
Thu 12 Nov 2009 10:30:02 PM UTC, original submission:  

http://www.opentstreetmap.org
click the edit tab (you need an account)

click on the camera icon.
it'll load photo from openstreetphoto.org.
Loads of photos....

Gnash eats all memory.

The loaded XML is here:
http://www.openstreetphoto.org/openstreetphoto.kml

potlatch.as will call an attachMovie for each icon
in it.

http://svn.openstreetmap.org/applications/editors/potlatch/potlatch.as

Commenting out the attachMovie call fixes it.
Reducing the number of Icons in the XML (.kml) file also
fixes it.

The .kml is a static file so the proprietary player
loads the same thing and I dubt it suffer from the same
memory consumption or OSM users would have noticed

Sandro Santilli <strk>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #19267:  EarlyGC.patch added by strk (1KiB - text/x-diff - patch showing that running GC during AS execution can save lots of memory)
file #19250:  innocent6.as added by strk (373B - application/x-applix-spreadsheet - single loop version)
file #19249:  innocent5.as added by strk (395B - application/x-applix-spreadsheet - less variables, less opcode, final cleanup )
file #19248:  innocent4.as added by strk (286B - application/x-applix-spreadsheet - no 'var', no conditionals, reduced string length)
file #19247:  innocent3.as added by strk (457B - application/x-applix-spreadsheet - testcase with NO function in it)
file #19243:  innocent2.as added by strk (416B - application/x-applix-spreadsheet - simplified testcase)
file #19242:  innocent.as added by strk (479B - application/x-applix-spreadsheet)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mindbooster (Voted in favor of this item)
  • -email is unavailable- added by mindbooster (Posted a comment)
  • -email is unavailable- added by annaanna1 (Posted a comment)
  • -email is unavailable- added by lindatan (Posted a comment)
  • -email is unavailable- added by bwy (Posted a comment)
  • -email is unavailable- added by strk (potlatch author..)
  • -email is unavailable- added by strk (Submitted the item)
  •  

    There are 50 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2010-07-13 mindbooster Carbon-Copy- Added mindbooster
    2009-12-14 strk Attached File- Added EarlyGC.patch, #19267
    2009-12-13 strk Attached File- Added innocent6.as, #19250
    2009-12-13 strk Attached File- Added innocent5.as, #19249
    2009-12-13 strk Attached File- Added innocent4.as, #19248
    2009-12-13 strk Attached File- Added innocent3.as, #19247
    2009-12-12 strk Attached File- Added innocent2.as, #19243
    2009-12-12 strk Attached File- Added innocent.as, #19242
    2009-11-12 strk Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code