Memory not consistently being released

Aside from circular references, it is difficult to leak memory with a reference counted system. What you are describing does not sound like a problem with reference counting, it sounds more like a leak of the menus, the delegates, or both.

When you say that this fails on the device, which device and firmware version are you referring to?

The fenix5 is running 12.40, the VA-HR is running 5.00

On further experimentation, I agree the issue seems to be a memory leak from the menu or delegates with the following observation:
The "select a mark" feature of the app, offers a two-level menu initiated from the Menu gesture (long-press on Up button or long-tap on the VA-3).
The first menu, (Screens option) offers options of various app functions, including, as option 3, "Navigation".
The Navigation option from the above menu offers another menu (marks menu) with options of the available marks.
Following these steps four or five times on either watch will cause an "Out of Memory" error on both watches, but not on the sim.
On the VA-HR and other touch screen devices, there is an option from the touch screen that directly opens the "marks menu" described above.
I am unable to replicate the memory leak using on the vahr using the second option.
I'll post the code behind the two-level menu system later as I'm off to do some on-water testing right now

I think I have got to the bottom of this.
It's all to do with how MonkeyC handles a menu (calling menu) that opens another menu (called menu) and whether popView() is executed from the calling menu.
I have determined that if you don't popView() from the calling menu, sometimes MC releases the memory held by the opening menu, sometimes it doesn't.
It seems the SDK sim always releases the calling menu's memory but the device sometimes does, sometimes doesn't, and that's where the inconsistent behaviour had me baffled.

The solution appears to be to always popView() in the calling menu's behaviorDelegate before executing the pushView to the called menu.

I am a bit surprised to find that the Ui.menu consumes around 7Kb.

It would be nice to be able to interrogate the number of views on the stack (Ui.countViews() ?) , as another way of managing the views in a layered app.

It seems the SDK sim always releases the calling menu's memory but the device sometimes does, sometimes doesn't, and that's where the inconsistent behaviour had me baffled.

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.

The solution appears to be to always popView() in the calling menu's behaviorDelegate before executing the pushView to the called menu.

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 am a bit surprised to find that the Ui.menu consumes around 7Kb.

Looking at the code you posted, you're creating a menu with up to 15 items. Each Menu holds an array of items, and each item holds a string and the id that you pass to the initializer. In addition to that, you allocate an array of your own (marks) where each array element is a Dictionary with three key/value pairs, with the keys being String and the values most likely being a String and two Doubles.

I had to do some testing, but an empty Dictionary is 132 bytes, a 3-character string is 28 bytes, and a Double is 28 bytes (not looking at the debugger, but determining the size via averages). So for 15 Dictionaries, each with 3 String keys of length 3, and two values of type Double, you're looking at..15 * (132 + ((3 * 28) + (2 * 28))) = 15 * (132 + 84 + 56) = 4110 bytes.

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

using Toybox.Application;
using Toybox.WatchUi;
using Toybox.Lang;

class MenuTestMenu extends WatchUi.Menu
{
function initialize(depth) {
Menu.initialize();

Menu.setTitle(Lang.format("Menu $1$", [ depth ]));
Menu.addItem(Rez.Strings.push_menu, :push_menu);
Menu.addItem(Rez.Strings.pop_menu, :pop_menu);
Menu.addItem(Rez.Strings.just_return, :just_return);
}
}

class MenuTestMenuDelegate extends WatchUi.MenuInputDelegate
{
hidden var mDepth;

function initialize(depth) {
MenuInputDelegate.initialize();
mDepth = depth;
}

function onMenuItem(item) {
if (item == :push_menu) {
WatchUi.pushView(new MenuTestMenu(mDepth + 1), new MenuTestMenuDelegate(mDepth + 1), WatchUi.SLIDE_UP);
} else if (item == :pop_menu) {
WatchUi.popView(WatchUi.SLIDE_DOWN);
}
}
}

class MenuTestDelegate extends WatchUi.BehaviorDelegate
{
function initialize() {
BehaviorDelegate.initialize();
}

function onMenu() {
WatchUi.pushView(new MenuTestMenu(0), new MenuTestMenuDelegate(0), WatchUi.SLIDE_UP);
return true;
}
}

class MenuTestView extends WatchUi.View
{
function initialize() {
View.initialize();
}

function onLayout(dc) {
setLayout(Rez.Layouts.MainLayout(dc));
}
}

class MenuTestApp extends Application.AppBase
{
hidden var mTimer;

function initialize() {
AppBase.initialize();
System.println("timestamp, used_memory");
}

function onStart(params) {
mTimer = new Timer.Timer();
mTimer.start(self.method(:onTimer), 3000, true);
}

function onStop(params) {
mTimer.stop();
mTimer = null;
}

function getInitialView() {
return [ new MenuTestView(), new MenuTestDelegate() ];
}

function onTimer() {
var systemStats = System.getSystemStats();
if (systemStats != null) {
var clockTime = System.getClockTime();
System.println(Lang.format("$1$:$2$:$3$, $4$", [
clockTime.hour,
clockTime.min.format("%02u"),
clockTime.sec.format("%02u"),
systemStats.usedMemory
]));
}
}
}


I ran this on a fenix5xplus (I don't have my vivoactive_hr or fenix5 handy), and here is the output I got from using the menu to push a few menus (via Push Menu), then pop a view views (via Just Return), then push a few menus (via Push Menu) and then pop a few (via the back button). I'm not seeing a memory leak at all.

timestamp, used_memory
19:41:39, 21136
19:41:42, 22040
19:41:45, 23056
19:41:48, 23056
19:41:51, 24080
19:41:54, 25096
19:41:57, 26120
19:42:00, 26120
19:42:03, 25120
19:42:06, 25120
19:42:09, 24120
19:42:12, 23120
19:42:15, 23120
19:42:18, 22120
19:42:21, 21248
19:42:24, 22120
19:42:27, 23120
19:42:30, 24120
19:42:33, 24120
19:42:36, 25120
19:42:39, 26120
19:42:42, 26120
19:42:45, 27136
19:42:48, 26136
19:42:51, 26136
19:42:54, 25136
19:42:57, 24136
19:43:00, 24136
19:43:03, 23136
19:43:06, 23136
19:43:09, 22136
19:43:12, 21264
19:43:15, 21264


I'm seeing about 1000 bytes for each menu I push, where a menu is a short string title, three menu item labels and values, the menu itself, and the delegate.

While I'm here, just a note about memory efficiency.. In your app, you're creating the same string keys for the dictionaries over and over again. This is a big waste of memory...

for (var i = 0; i < loadedMarks.size() && i < 15; i++) {
marks.add({
"name" => loadedMarks["name"],
"lat" => loadedMarks["lat"],
"lon" => loadedMarks["lon"]
});
}
[/code]

In this code, you re-create strings "name", "lat", and "lon" over and over again, each one of them taking at least 28 bytes. If you just created three strings and re-used them, you'd save a bunch of memory.

// as long as you don't have too many references to them, you could make these global const.
var name = "name";
var lat = "lat";
var lon = "lon";

for (var i = 0; i < loadedMarks.size() && i < 15; i++) {
marks.add({
name => loadedMarks[name],
lat => loadedMarks[lat],
lon => loadedMarks[lon]
});
}
[/code]

You could also just use an integer, or even a symbol (provided you don't put them into the object store)

// these can be global const.. you don't need to worry about the number of references
const NAME = 1;
const LAT = 2;
const LON = 3;

for (var i = 0; i < loadedMarks.size() && i < 15; i++) {
marks.add({
NAME => loadedMarks["name"],
LAT => loadedMarks["lat"],
LON => loadedMarks["lon"]
});
}
[/code]

Travis

I thought Monkey C folded duplicate strings... Or maybe I imagined reading that once.

Just my two cents, but to save even more memory, you could either:
- Use an array of 3 items instead of a dictionary for each element of marks[]
marks.add([
loadedMarks["name"],
loadedMarks["lat"],
loadedMarks["lon"]
]);

// elsewhere...

var name = marks[0];
var lat = marks[1];
var lon = marks[2]; [/code]

- Or flatten marks completely:
marks.addAll([
loadedMarks["name"],
loadedMarks["lat"],
loadedMarks["lon"]
]);

// elsewhere...

var name = marks[i*3];
var lat = marks[i*3 + 1];
var lon = marks[i*3 + 2]; [/code]

As discussed, dictionaries are very expensive in terms of memory, and arrays within arrays are expensive too (although not as bad as dictionaries within arrays). If you can just have one array (and 0 dictionaries) for marks[], that's a win.

I'm all for "pretty", descriptive and elegant code (I fought many battles at my workplace IRL over this), but when memory is this limited, I'm happy to write code that's as ugly and opaque as possible so it'll fit.

The most beautiful, reusable and self-documenting code in the world is no good if it won't run.

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.

This is something I've previously seen in the forums (it seemed to cause some discussion) that I wish were explicitly documented.... I guess if it works that way in the sim and the device, we can't ask for anything more.

I thought Monkey C folded duplicate strings... Or maybe I imagined reading that once.

When serializing and deserializing data the strings are de-duplicated. i.e., if you get a web response that has a lot of json with duplicate keys, they will be de-duped for memory efficiency.

Just my two cents, but to save even more memory, you could either:

Certainly. I was trying to point out a change that would result in no discernible difference in the result or the code but would give an efficiency gain. Obviously going with a array(s) would help, but it does (as you've pointed out) make the code uglier and slightly more difficult to maintain.

As discussed, dictionaries are very expensive in terms of memory

That might change... We've found a bug that causes dictionary to allocate more memory than it is supposed to have. Once this is fixed, they should just go back to pretty expensive from very expensive.. :-)