Memory not consistently being released

Travis.ConnectIQ Thanks for the explanation re: deduplication.

Totally understandable as far as your example code goes, and I totally knew what you were going for ;). There's always that balance between ugly code that's super-efficient (and completely unmaintainable) and beautiful code that is bloated.

In this case, if it were me, I would go with the most efficient method, especially if it were possible to put the code that sets and gets marks[] very close together (perhaps in auxiliary helper functions, such as setMarkItem() and getMarkItem()). By localizing the ugliness to a couple of places, you could have the best of both worlds, or at least a reasonable compromise.

e.g. In this case, nobody besides getMarkItem() and setMarkItem() need to know the internals of the marks array. This could be nicer by making all of this into a class, but then you're getting more inefficient again. (Although as long as it's a single class and not an array of classes, it wouldn't be so bad.)
// Assumes marks is a non-null array type
//
// Example usage:
// var marks = [];
// for (i = 0; i < 15; i++)
// {
// setMarkItem(marks, i, loadedMarks["name"], loadedMarks["lat"], loadedMarks["lon"]);
// }

function setMarkItem(marks, idx, name, lat, lon)
{
// this part is inefficient. Can get around this by initializing marks appropriately. (e.g. var marks = new [15 * 3])
if (marks.size() <= idx*3) {
marks.addAll(new [marks.size() - idx*3 + 1]); // grow array (fill with nulls)
}
marks[idx*3] = name;
marks[idx*3 + 1] = lat;
marks[idx*3 + 2] = lon;
}

// Assumes marks is a non-null array type
//
// Returns
/ null, if idx is out of range
// otherwise: [name, lat, lon]
function getMarkItem(marks, idx)
{
if (marks.size() <= idx*3) {
return null;
}
return [marks[idx*3], marks[idx*3 + 1], marks[idx*3 + 2];
}
[/code]

I take on board the great comments about my frivolous use of strings and will mend my ways, thanks. I'm keeping Dictionaries here for compatibility with some legacy code (Pebble, Garmin, Apple, back to Garmin!).

However, I don't think we've fully addressed the issue of the memory release inside the menus:

I've looked at the sim and the device code, and they appear to do almost exactly the same thing... If you return from MenuInputDelegate.onMenuItem() without having pushed a view (any type of view), the system will automatically pop the current menu. This behavior may seem weird, but it has been this way since ConnectIQ 1.0.

I'm sure you have, but my contrary evidence is pretty clear. I could crash the app reliably on the device but not on the sim - time after time, using identical UI.
And then there's the question of the inconsistency I have found in the release of memory ....

I wrote some test code, and verified that calling WatchUi.popView() from onMenuItem() does indeed pop two menu items as I suggested.

I really appreciate your work, but there must be something else happening in my code.

If you call popView() from onMenuItem(), then two views will be popped.. The view that you are explicitly popping and the view that the system is automatically popping. Are you certain that you aren't pushing multiple menus at once?

I'm pretty sure I'm not popping two views. Since adding the popView in the calling menu, the app is performing to plan. No unexpected popped views, so that's the work-around I'm sticking with for now.

I'm keeping Dictionaries here for compatibility with some legacy code (Pebble, Garmin, Apple, back to Garmin!).

Sure, it's up to you, it was just a suggestion. My only point is that it seems no internal code of your needs to store 15 dictionaries in an array, unless I am missing something. I haven't really looked at your code closely, but it seems to me that you would only need to deal with one dictionary at a time (the selected menu item?)

By eliminating dictionaries inside of marks[], you could save something like 15 * 132 bytes.

EDIT: Removed suggestion from this post and made completely different (and much simpler) suggestion in following post.

As a matter of fact, either way, it seems you are expending a lot of effort, code and memory just to recreate either loadedMarks or loadedCourses in a common format: marks. The data is already there, right? Why do you need to recreate it when you can change the way it's accessed?

This is just a suggestion. I won't be offended if you don't like it :P.

"New" code is bolded.


class navOptionsMenuDelegate {
//var marks;
var marksOrCourses = null; // stores either loadedMarks or loadedCourses

enum {
MARKS,
COURSES
}

var marksOrCoursesType = MARKS;

// Return mark or course item in common dictionary format
function getMarksOrCourseItem(idx) {
if (marksOrCoursesType == MARKS) {
return { "name" => marksOrCourses[idx]["name"],
"lat" => marksOrCourses[idx]["lat"],
"lon" => marksOrCourses[idx]["lon"] };
} else { // courses
return { "name" => marksOrCourses[idx][0],
"lat" => marksOrCourses[idx][1],
"lon" => marksOrCourses[idx][2] };
}
}

// ...

}


function cansCourseSelected() {

// ...

if (navMode == e_cans) {
for (var i = 0; i < loadedMarks.size() && i < 15; i++) { // populate
s_menu_layer.addItem(loadedMarks["name"]
, i // for menu selection
);
}_navOptionsMenuDelegate.marksOrCourses = loadedMarks;
_navOptionsMenuDelegate.marksOrCoursesType = _navOptionsMenuDelegate.MARKS;
}
else if (navMode == e_course) {
for (var i = 0; i < loadedCourses.size(); i++) { // populate
s_menu_layer.addItem(loadedCourses[0]
, i // for menu selection
);
};_navOptionsMenuDelegate.marksOrCourses = loadedCourses;
_navOptionsMenuDelegate.marksOrCoursesType = _navOptionsMenuDelegate.COURSES;
}

//_navOptionsMenuDelegate.marks = marks;
s_menu_layer.addItem("W/L Marks", 99);
Ui.pushView(s_menu_layer, _navOptionsMenuDelegate, Ui.SLIDE_IMMEDIATE);
}
[/code]

EDIT: I realize this isn't the greatest design because navOptionsMenuDelegate has knowledge of the internal structure of loadedMarks or loadedCourses. So getMarksOrCourseItem() could moved somewhere else, where it makes more sense. You could even put all of this in a mini-class, so the code is nice and maintainable.

But the general idea would save you lots of memory, I think....

I'm sure you have, but my contrary evidence is pretty clear.

Sure. Like I said, I'd like to see a test case. I provided code (pasted above) that demonstrates calling popView() does indeed pop two views (on a fenix5xplus and in the simulator), and that memory does not appear to leak when pushing menus and not explicitly popping them. You could easily verify the behavior by building the code and testing it on your device.

I could crash the app reliably on the device but not on the sim - time after time, using identical UI.

Yes. I agree there is a discrepancy here, but until we pinpoint the source, this is all speculation.

And then there's the question of the inconsistency I have found in the release of memory ....

There is a high probability that this inconsistency seen in your app is the source of the crashes... the two problems are one and the same.

I'm pretty sure I'm not popping two views. Since adding the popView in the calling menu, the app is performing to plan. No unexpected popped views, so that's the work-around I'm sticking with for now.

That may be your workaround, which is fine, but until we can reproduce a problem here it is difficult for us to determine if there is a problem with your code, with the device firmware, or with the virtual machine. And without being able to determine the source, it will be impossible for us to know what needs to be fixed.

Travis

Sure. Like I said, I'd like to see a test case. I provided code (pasted above) that demonstrates calling popView() does indeed pop two views (on a fenix5xplus and in the simulator), and that memory does not appear to leak when pushing menus and not explicitly popping them. You could easily verify the behavior by building the code and testing it on your device.

Yes. I agree there is a discrepancy here, but until we pinpoint the source, this is all speculation.

There is a high probability that this inconsistency seen in your app is the source of the crashes... the two problems are one and the same.

That may be your workaround, which is fine, but until we can reproduce a problem here it is difficult for us to determine if there is a problem with your code, with the device firmware, or with the virtual machine. And without being able to determine the source, it will be impossible for us to know what needs to be fixed.

Travis

OK, It's a challenge!
I am confident that I am seeing a memory leak from a two level menu in my app, but it's much too unwieldy to use as a test case as it requires a great deal of set-up.

I'll see if I can build a test case, It'll take a bunch of hours out of my development time which I'll fit in on over the next few days.
Stay tuned.

You should be able to start with the sample code I posted above. I'm still tinkering with this nearly a day later, so you're not the only one dedicating time to it. :)

You should be able to start with the sample code I posted above. I'm still tinkering with this nearly a day later, so you're not the only one dedicating time to it. :)

Rather than tinker with your code that disproves my theory, I have tinkered with my code that demonstrates one of my contentions: that the sim behaves differently from the device. The sim releases memory and device does not.

using Toybox.Application as App;

using Toybox.WatchUi as Ui;

using Toybox.Graphics as Gfx;

var mainViewDelegate;

var mainView;


class main extends App.AppBase {

function initialize( ) {
AppBase.initialize( );

}

function getInitialView( ) {

mainView = new MainView() ;

mainViewDelegate = new MainViewDelegate();

return [mainView, mainViewDelegate ];

}

}




class MainView extends Ui.View {

function cansCourseSelected(){

var s_menu_layer = new Ui.Menu();

s_menu_layer.setTitle("Second Menu");

s_menu_layer.addItem("2nd Menu Opt.", 1);

Ui.pushView(s_menu_layer, new secondMenuDelegate(), Ui.SLIDE_IMMEDIATE);

}



function onUpdate(dc) {

dc.setColor(Gfx.COLOR_BLACK, Gfx.COLOR_BLACK);

dc.clear( );

dc.setColor(Gfx.COLOR_WHITE, Gfx.COLOR_TRANSPARENT);

dc.drawText(

dc.getWidth( ) / 2,

dc.getHeight( ) *1/3,

Gfx.FONT_MEDIUM,

"Press Menu",

Gfx.TEXT_JUSTIFY_CENTER|Gfx.TEXT_JUSTIFY_VCENTER

);

var currentMemory = System.getSystemStats().usedMemory;

dc.drawText(

dc.getWidth( ) / 2,

dc.getHeight( ) *3/ 4,

Gfx.FONT_MEDIUM,

"Memory:" + currentMemory,

Gfx.TEXT_JUSTIFY_CENTER|Gfx.TEXT_JUSTIFY_VCENTER

);

}

}

class MainViewDelegate extends Ui.BehaviorDelegate {

function onMenu() {

var s_menu_layer = new Ui.Menu();

s_menu_layer.setTitle("First Menu");

s_menu_layer.addItem("1st Menu Opt. ",:navOption);

Ui.pushView(s_menu_layer, new firstMenuDelegate(), Ui.SLIDE_IMMEDIATE);

}

}

class secondMenuDelegate extends Ui.MenuInputDelegate {

function initialize() {

}

function onMenuItem(item) {

Ui.popView(0);

/*do application stuff based on the selected option */

Ui.switchToView(mainView , mainViewDelegate, 0); // back to initial view

}

}







class firstMenuDelegate extends Ui.MenuInputDelegate {

function onMenuItem(item) {

if (item == :navOption){

//Ui.popView(0); //CONTENTIOUS ITEM

var s_menu_layer = new Ui.Menu();

s_menu_layer.setTitle("Second Menu");

s_menu_layer.addItem("2nd Menu Opt.", 1);

Ui.pushView(s_menu_layer, new secondMenuDelegate(), Ui.SLIDE_IMMEDIATE);

}




}

}








The test procedure is to run the code on the SDK sim and on a device (I used a vahr)


Loop 1. Menu > select "1st Menu Opt". >Select "2nd Menu opt."

Loop 2. Menu > select "1st Menu Opt". >Select "2nd Menu opt."

Loop 3. Menu > select "1st Menu Opt". >Select "2nd Menu opt."

etc….

Expected behaviour:The sim should perform the same as the device.

Actual behaviour:

Running on the sim, the displayed memory remains the same however many times you go round the loop.

On the device, each time you go round the loop the displayed memory increases.



What appears to be happening is:

The mainViewDelegate's onMenu() is pushing firstMenu, leaving 2 views on the stack: mainView and firstMenu.

The firstMenuDelegate's onMenuItem() is pushing secondMenu on to stack, leaving three views on the stack: mainView, firstMenu, and secondMenu.

In the secondMenu's onMenuItem()

the popView() is popping the secondMenu off the stack leaving mainView and firstMenu on the stack.

the switchToView is switching from firstMenu to mainView, leaving two copies of mainView on the stack.


Each time the cycle is repeated, another copy of mainView is added to the stack

The simulator's View Memory display confirms that the number of views on the view stack increases at the start of each loop.

(The multiple copies of the mainView can be seen as you back out from the main view.)


The odd thing is that the multiple copies of mainView don't appear to consume any memory on the sim, whilst they do on the device, as the memory value increases on the device in each loop while it remains constant on the sim.


This is what lead me to the conclusion of a memory leak on the device, as I was developing on the sim but observing Out of Memory errors in the field on the device.


Not having a spec of what should be the behavior makes it hard to determine which one is correct.


Bui in neither case is the popView pop two views as you suggested.

(apologies for the double line spacing - something to do with copy/pase on a Mac)

Wow, great find!

This is just my two cents, but I think the simulator is wrong, as the View stack is clearly getting larger while not one additional byte of memory is consumed, according to the simulator. This would seem to be impossible, as there the View stack has to be able to pop each and every one of those mainView references (even if you never do), so it has to track them somehow. Unless of course there's some clever code in the View stack which just says: "Here's X references to mainView."

Perhaps the bug is that the sim is able to do that optimization while the device is not. I do expect that the sim (theoretically) would crash eventually, after 256, 65536, 2[SUP]31[/SUP], 2[SUP]32[/SUP] or maybe even 2[SUP]64[/SUP] calls to switchView. Obviously those last three are impossible to test, if you could go past 65536.

View Stack aside, since you're switching to the same instance of mainView every time (instead of creating a new instance to switch to), if you changed the state of mainView, I assume that any change to one of references on the stack would change all of them, so in one sense it's not that surprising that memory usage isn't increasing. IOW, you don't seem to have multiple copies of mainView, but multiple references to one copy of mainView.

EDIT: I did the test on Windows and looked at the memory usage of the simulator (in Windows). The usage doesn't seem to change/grow no matter how big the stack gets, although I didn't automate it, so I didn't get past about 30 or so references to mainView on the stack. But that would seem to indicate the sim really is doing an optimization trick so all those duplicate references really do take the same total amount of memory, no matter how many you have.

Yes I did spend a few minutes attempting to change the state of mainView, and can confirm that the change was reflected in each of the instances, confirming that we have references not multiple instances. In the Memory Usage window, you'll find the the view stack is a stack of the same object (by number).