Announcement

Collapse
No announcement yet.

Methods assigned to variables not being released from memory, or something like that.

Collapse
X
  • Time
  • Show
Clear All
new posts

  • Methods assigned to variables not being released from memory, or something like that.

    Watch the memory usage climb, in the simulator, each time you push the enter key.
    Code:
    using Toybox.Application as App;
    using Toybox.WatchUi as Ui;
    using Toybox.Graphics as Gfx;
    using Toybox.System as Sys;
    class MallocApp extends App.AppBase {
        function getInitialView() {
            return [ new MallocView(), new MallocDelegate() ];
        }
    }
    class MallocView extends Ui.View {
        function onUpdate(dc) {
            dc.setColor(Gfx.COLOR_WHITE, Gfx.COLOR_BLACK);
            dc.clear();
            dc.drawText(dc.getWidth()/2,dc.getHeight()/2, 
            	Gfx.FONT_LARGE, "Malloc View", Gfx.TEXT_JUSTIFY_CENTER);        
        }
    }
    class Malloc {
    	var mAlloc = new [1000];
    	var mFunc;
    	function initialize(){
    		mFunc = method(:doNothing);
    	}
    	function doNothing(){	
    	}
    }
    class Mfree {
    	var mAlloc = new [1000];
    	var mFunc;
    	function initialize(){
    		mFunc = null;
    	}
    	function doNothing(){	
    	}
    }
    class MallocDelegate extends Ui.InputDelegate {
        function onKey(event){    	
        	var key = event.getKey();    	
            if(Ui.KEY_ENTER == key){
            	Sys.println("Enter = add KB's");
            	var myClass = new Malloc();
            }        
            else if(key == Ui.KEY_ESC){
            	Sys.println("Esc = don't add KB's");
            	var myClass = new Mfree();
            }
        }
    }
    I think one thing, yet type another.

  • #2
    The callable object returned by method() keeps a reference to the object that it is to be invoked on (self in this case). This prevents the object from being destroyed and deallocated while it could still be referenced via the callable.

    I'm not sure what your original code looks like, so it is difficult to suggest a reasonable workaround.

    Comment


    • #3
      Yes, if I change the code a little it will clean up after itself in the sample.
      Code:
      function initialize(){
      		mFunc = method(:doNothing);
      		mFunc = null;
      	}
      I am applying different methods to the buttons, depending on what input I need at the time. So whereas I can clean up in the sample with mFunc = null, my program doesn't seem to work in the same way.

      Here is the code that creates the class from within the initial app....
      Code:
      function onKeyEnter(){	
      		var test = new Test();	
      		Ui.pushView(test.getView(), new KeyDelegate(test), Ui.SLIDE_IMMEDIATE);
          }
      and here is my exit code from the test class.....
      Code:
      function killAndPop(){
      		reset();	
      		mOnKeyEnter = null;
      		mOnKeyEsc = null;	
      		Ui.popView(Ui.SLIDE_DOWN);
      	}
      My intention was to keep the code separate from an extended Ui.View class, which is why test calls it's own Ui.View. I'm self taught, so maybe that's just bad coding practice? In trying to find the problem I had removed every other variable etc, so it seem's to me the problem is specific to assigning the method to the variable. But as you can see what works in the sample doesn't work in my app. Maybe the pushing and popping is adding a complication?
      I think one thing, yet type another.

      Comment


      • #4
        Here is a sample that shows the problem in the way that I am using it.
        Code:
        using Toybox.Application as App;
        using Toybox.WatchUi as Ui;
        using Toybox.Graphics as Gfx;
        using Toybox.System as Sys;
        
        class Main extends App.AppBase {
            var mView;
            function onStart() {
            	mView = new MainView();
            }
            function getInitialView() {
                return [new MainView(), new KeyDelegate(self)];
            }
            function onKeyEnter(){
            	var test = new Test();
            	Ui.pushView(test.getView(), new KeyDelegate(test), Ui.SLIDE_IMMEDIATE);
            }    
        }
        class Test {
            var mTestView;
            var mOnKeyEnter;
            var mOnKeyEsc;
            function initialize(){
            	mOnKeyEnter = method(:doNothing);
        	mOnKeyEsc = method(:killAndPop);
            	mTestView = new TestView();
        	}
        	function doNothing(){
        	}
        	function getView(){
        		return mTestView;
        	}
        	function killAndPop(){
        		mTestView = null;
        		mOnKeyEnter = null;
        		mOnKeyEsc = null;
        		Ui.popView(Ui.SLIDE_IMMEDIATE);
        	}
        	function onKeyEnter(){
        		mOnKeyEnter.invoke();
        	}
        	function onKeyEsc(){
        		mOnKeyEsc.invoke();
        	}
        	
        }
        class MainView extends Ui.View {
            function onUpdate(dc) {
                dc.setColor(Gfx.COLOR_WHITE, Gfx.COLOR_BLACK);
                dc.clear();
                dc.drawText(dc.getWidth()/2,dc.getHeight()/2, 
                	Gfx.FONT_LARGE, "Main View", Gfx.TEXT_JUSTIFY_CENTER);        
            }
        }
        class TestView extends Ui.View {
            function onUpdate(dc) {
                dc.setColor(Gfx.COLOR_WHITE, Gfx.COLOR_BLACK);
                dc.clear();
                dc.drawText(dc.getWidth()/2,dc.getHeight()/2, 
                	Gfx.FONT_LARGE, "Test View", Gfx.TEXT_JUSTIFY_CENTER);        
            }
        }
        class KeyDelegate extends Ui.InputDelegate {
            hidden var parent;    
            function initialize(parent){
            	self.parent = parent;
            }    
            function onKey(event) {    	
            	var key = event.getKey();    	
                if(Ui.KEY_ENTER == key){
                	if(parent has :onKeyEnter){
                		parent.onKeyEnter();
                	}
                }
                else if(key == Ui.KEY_ESC){
                	if(parent has :onKeyEsc){
                		parent.onKeyEsc();
                	}
                }        
            }
        }
        I think one thing, yet type another.

        Comment


        • #5
          I'll address your original question first, and then discuss your implementation a bit further. I think you may arrive at a better design after talking it over.

          So it appears that there is a memory leak here, but it has nothing to do with the callbacks as you originally expected. It relates to the use of the has operator. If you comment out the if has checks in the KeyDelegate and add an empty implementation of Main.onKeyEsc, you will see that memory is no longer leaked. This indicates that the leak is in the MonkeyC implementation and not your code.

          So now that is out of the way, onto some quick notes. Some of this might be invalid because your test program isn't the same as your real program...
          1. In Main.onStart(), you allocate a new MainView and assign it to mView, but you don't appear to use it at all. You create a new view in Main.getInitialView(). One of these is unnecessary.
          2. Your InputDelegate isn't returning a boolean value as it should. If the input is handled, you should return true.


          And now onto design discussion. I'm still not completely sure I see what you are trying to do. It seems that you are trying to push the key handling logic out of KeyDelegate while still trying to avoid putting the logic into each Ui.View. This is fine, I just want to be sure I understand your goals. Why are you trying to keep the input handling logic out of the view?

          If the behavior of each of the views is static, then it seems the code would be more simply written by putting the key handling logic into the view...

          Code:
          using Toybox.Application as App;
          using Toybox.WatchUi as Ui;
          using Toybox.Graphics as Gfx;
          using Toybox.System as Sys;
          
          class TestApp extends App.AppBase {
          
              function onStart() {
              }
          
              function getInitialView() {
                  var view = new MainView();
                  return [ view, new KeyDelegate(self) ];
              }
          }
          
          class MainView extends Ui.View {
              function onUpdate(dc) {
                  dc.setColor(Gfx.COLOR_WHITE, Gfx.COLOR_BLACK);
                  dc.clear();
                  dc.drawText(dc.getWidth()/2,dc.getHeight()/2,
                              Gfx.FONT_LARGE, "Main View", Gfx.TEXT_JUSTIFY_CENTER);
              }
          
              function onKeyEnter() {
                  var view = new TestView();
                  Ui.pushView(view, new KeyDelegate(test), Ui.SLIDE_IMMEDIATE);
                  return true;
              }
          
              function onKeyEnter() {
                  return false;
              }
          }
          
          class TestView extends Ui.View {
              function onUpdate(dc) {
                  dc.setColor(Gfx.COLOR_WHITE, Gfx.COLOR_BLACK);
                  dc.clear();
                  dc.drawText(dc.getWidth()/2,dc.getHeight()/2,
                              Gfx.FONT_LARGE, "Test View", Gfx.TEXT_JUSTIFY_CENTER);
              }
          
              function onKeyEnter() {
                  return false;
              }
          
              function onKeyEsc() {
                  Ui.popView(Ui.SLIDE_IMMEDIATE);
                  return true;
              }
          }
          
          class KeyDelegate extends Ui.InputDelegate {
              hidden var parent;
          
              function initialize(parent) {
                  self.parent = parent;
              }
          
              function onKey(event) {
                  var key = event.getKey();
                  if(Ui.KEY_ENTER == key) {
                      return parent.onKeyEnter();
                  }
                  else if(key == Ui.KEY_ESC) {
                      return parent.onKeyEsc();
                  }
          
                  return false;
              }
          }
          If there are cases where the behavior of the view needs to vary and using a flag is just getting too complicated, It might make sense to put some intelligence into the input delegate, and make different input delegates. This puts all of the display logic in the view and the input handling logic into the delegate as it seems the system was designed.

          Code:
          using Toybox.Application as App;
          using Toybox.WatchUi as Ui;
          using Toybox.Graphics as Gfx;
          using Toybox.System as Sys;
          
          class TestApp extends App.AppBase {
          
              function getInitialView() {
                  var view = new MainView();
                  return [ view, new MainDelegate() ];
              }
          }
          
          class KeyDelegate extends Ui.InputDelegate {
          
              function onKey(event) {
                  var key = event.getKey();
                  if(Ui.KEY_ENTER == key) {
                      return onKeyEnter();
                  }
                  else if (Ui.KEY_ESC == key) {
                      return onKeyEsc();
                  }
                  // else if (Ui....)
          
                  return false;
              }
          
              function onKeyEnter() {
                  return false;
              }
          
              function onKeyEsc() {
                  return false;
              }
          }
          
          class MainDelegate extends KeyDelegate {
          
              function onKeyEnter() {
                  var view = new TestView();
                  Ui.pushView(view, new TestDelegate(), Ui.SLIDE_IMMEDIATE);
              }
          }
          
          class TestDelegate extends KeyDelegate {
              function onKeyEsc() {
                  Ui.popView(Ui.SLIDE_IMMEDIATE);
              }
          }
          
          class MainView extends Ui.View {
              function onUpdate(dc) {
                  dc.setColor(Gfx.COLOR_WHITE, Gfx.COLOR_BLACK);
                  dc.clear();
                  dc.drawText(dc.getWidth()/2,dc.getHeight()/2,
                              Gfx.FONT_LARGE, "Main View", Gfx.TEXT_JUSTIFY_CENTER);
              }
          }
          
          class TestView extends Ui.View {
              function onUpdate(dc) {
                  dc.setColor(Gfx.COLOR_WHITE, Gfx.COLOR_BLACK);
                  dc.clear();
                  dc.drawText(dc.getWidth()/2,dc.getHeight()/2,
                              Gfx.FONT_LARGE, "Test View", Gfx.TEXT_JUSTIFY_CENTER);
              }
          }
          There is still a third option. If the behavior of your view needs to change based on some other external criteria, or you want to separate your implementation completely from the Ui.View and Ui.InputDelegate, you could employ the state pattern.

          Code:
          class TestState
          {
              function onKeyEnter() {
                  return false;
              }
          
              function onKeyEsc() {
                  return false;
              }
          
              function doSomethingStateSpecific()  {
              }
          }
          
          class TestState1 extends TestState
          {
              function onKeyEsc() {
                  Ui.popView(Ui.SLIDE_IMMEDIATE);
                  return true;
              }
          
              function doSomethingStateSpecific()  {
                  Sys.println("TestState1.doSomethingStateSpecific");
              }
          }
          
          class TestState2 extends TestState
          {
              function onKeyEsc() {
                  Ui.popView(Ui.SLIDE_IMMEDIATE);
                  return true;
              }
          
              function doSomethingStateSpecific()  {
                  Sys.println("TestState2.doSomethingStateSpecific");
              }
          }
          
          class TestView extends Ui.View {
              hidden var state;
          
              function setState(state) {
                  self.state = state;
              }
          
              function onUpdate(dc) {
                  dc.setColor(Gfx.COLOR_WHITE, Gfx.COLOR_BLACK);
                  dc.clear();
                  dc.drawText(dc.getWidth()/2,dc.getHeight()/2,
                              Gfx.FONT_LARGE, "Test View", Gfx.TEXT_JUSTIFY_CENTER);
              }
          
              function onKeyEsc() {
                  state.onKeyEsc();
              }
          
              function onKeyEnter() {
                  state.onKeyEnter();
              }
          
              function doSomething() {
                  state.doSomethingStateSpecific();
              }
          }
          
          
              // wherever you create a test view, you must specify the state that the view is in
              var view = new TestView();
              view.setState(new TestState1());
              Ui.pushView(view, new KeyDelegate(view), Ui.SLIDE_IMMEDIATE);
          IMO, using the state pattern here is overkill if all you want to do is to have different behavior for responding to key presses. You have to create a separate state for each input handling scenario, which is no better than creating a new KeyDelegate for each. If you really do need to have state specific behavior outside of the input handling, or you want to reuse that behavior, then maybe the state pattern makes sense.

          Comment


          • #6
            Thanks for all the input. Nice work locating the problem, I hadn't even considered the "has".

            In response to your q's 1 & 2.
            1/ I simply hadn't progressed to the point of coding the initial view yet. It will have multiple view choices.
            2/ I hadn't added the return because I didn't need it yet. I will try to remember to follow the original more closely in future.

            There were 2 main reasons why the code is kept away from the view.
            1/ I started testing off the MO2 sample, and my test code was already extending the Ant.GenericChannel class.
            2/ I can reuse the view in multiple ways, and I don't want to code it each time for the other uses, as I modified the design I would have to modify the code in multiple different places.
            I had also thought it was more oo to keep the view as an entity. Flawed logic?

            Even after consideration I still think my original concept will suit my needs best, which is to keep the test running whilst still being able to modify the behaviors of the buttons. This is largely based on not wanting to flash multiple new (different) views into the users vision. Pushing new delegates with the same view just seems wrong to me, as the test code will end up spread over too many different delegates. I hope I am not missing something totally obvious.

            So for now I will remove the "has" checking. It was there because my delegate included all the possible keys, and I could reuse it without modification. I will create specific delegates that handle the keys I intend to use. I am slowly reverting back to the drawing board to flesh out the handling logic, program flow, and other such stuff. I needed the mock-up to help me better understand the user interaction.
            Last edited by sharkbait_au; 01-02-2015, 07:14 PM.
            I think one thing, yet type another.

            Comment


            • #7
              Originally posted by TRAVIS.VITEK View Post
              This indicates that the leak is in the MonkeyC implementation and not your code.
              Travis, your post above indicates that if we use the has operator to check if a method is present in the current firmware version, that a memory leak will occur. I recently updated a few of my apps to be backward compatible using the has operator and this has now caused leaks to appear. Do you have any other suggestions on how we should use the has operator to check this? I specifically want to check if the System.phoneConnected property exists or not.

              If I use the has operator, my app slowly builds up memory usage until it run out of memory as below (without has, it remains on 46212)...

              Code:
              if (sysSet has :phoneConnected) {
              			if (sysSet.phoneConnected) {
              				ShowBTIcon(dc);
              			}
              		}
              63735
              64032
              64329
              64626
              64923
              Failed invoking <symbol>
              Out Of Memory Error
              @PC = 0x1000044f
              @PC = 0x1000044f
              @PC = 0x100002f1
              Out Of Memory Error

              Comment


              • #8
                Check it only once in constructor or onLayout method, not on every onUpdate call.

                Comment


                • #9
                  Exactly. Use the operator once, and cache the result.

                  Comment


                  • #10
                    Has this memory leak been resolved, or is it still present in the CiQ framework?

                    Seems like a language that supposedly has duck typing based polymorphism should probably have a functional, non-leaking `has` operator?

                    Comment


                    • #11
                      I'm fairly certain that the has leak was fixed some time ago.

                      Comment

                      Working...
                      X