Big update to prettier-extension-monkeyc

v2.0.55 is out.

This fixes https://github.com/markw65/prettier-extension-monkeyc/issues/7 by working around https://forums.garmin.com/developer/connect-iq/i/bug-reports/import-rez-or-using-rez-breaks-background-resources. The fix is conservative - if a :background function accesses Rez.Strings.Foo, the optimizer won't add the "import Rez;". It could be smarter - if Rez.Strings.Foo isn't marked with scope="background", then it can safely assume that the user knows it will never actually be called in the background, and could still optimize Rez. I'll probably add that later.

It also adds a couple more post build optimizations, which bring the post-build savings to ~360 bytes in my 11k project (up from ~290 in the previous release), on top of the ~700 byte savings from the source to source optimizer.

I found a bug a few days ago (so it was there before 2.0.55) I'm trying to create a minimal example to send it to you but the bug is basically that some code was removed from the optimized code:

I had this line of code, and it still works today:

flocsy🤠 said:

I found a bug a few days ago

I'm probably going to need a bit more to track this down, but...

• guageOnTop is a parameter to the function? And its explicitly Boolean?
• y is another local?
• how is ly used afterwards? exactly once? more? conditionally?

    hidden function drawHrGaugeHand(dc as Graphics.Dc, heartRate as Number, triangleColor as Graphics.ColorType, foregroundColor as Graphics.ColorType, triangleHeight as Number,
                z1 as Number, z5 as Number, pixelsPerHr as Float, gaugeOnTop as Boolean) as Void {
        var one = 1;
        var y = mGaugeY;
        var gaugeHeight1 = mGaugeH - one;
        var hrd = heartRate - z1;
        var hrx = (mGaugeX + (hrd < 0 ? -1 : (heartRate <= z5 ? hrd : z5 - z1 + one)) * pixelsPerHr).toNumber();
        dc.setColor(foregroundColor, foregroundColor);
        dc.drawLine(hrx, y, hrx, y + gaugeHeight1);

        dc.setColor(triangleColor, foregroundColor);
        var dy = gaugeOnTop ? one : -1;

        // old, working:
        // var ly = y + (gaugeOnTop ? gaugeHeight1 : 0);

        // new, doesn't work:
        var ly = y;
        if (gaugeOnTop) { // the whole if block is disappeared from the optimized code
            ly += gaugeHeight1;
        }

        var i = one;
        do {
            dc.drawLine(hrx - i, ly, hrx + i, ly);
            ly += dy;
            i++;
        } while (i <= triangleHeight);
    }

yes, gaugeOnTop is explicitly Boolean as you see,

y is a local variable, it's an optimization, instead of using a class variable 3 times

you can see how ly is used in the loop. it's read twice and increased once

LOL, ok, so whatever the bug is, it's still there and needs to be fixed, but now I have an even better optimization:

var yPlusGaugeHMinusOne = y + gaugeHeight1;
dc.drawLine(hrx, y, hrx, yPlusGaugeHMinusOne);
...
var ly = gaugeOnTop ? yPlusGaugeHMinusOne : y;

and here's one more potential to optimization BTW:

look at the end of the function:

        var i = one;
        do {
            dc.drawLine(hrx - i, ly, hrx + i, ly);
            ly += dy;
            i++;
        } while (i <= triangleHeight);

In the. optimized code we still create a variable for i, though it's kind of unnecessary, because pre_1 could have been used "as is", because it's not used later in the function:

    i = pre_1;
    do {
      dc.drawLine(
        z5 /*>hrx<*/ - i,
        heartRate /*>ly<*/,
        z5 /*>hrx<*/ + i,
        heartRate /*>ly<*/
      );
      heartRate /*>ly<*/ += z1 /*>dy<*/;
      i++;
    } while (i <= triangleHeight);

Even if it couldn't totally spare declaring the i (because it is being used before this in the optimized code) the assignment (i = pre_1) could still be spared.

flocsy🤠 said:

the assignment (i = pre_1) could still be spared

Yes. I said when I implemented the variable sharing that it was a greedy algorithm that wouldn't always come up with the optimal solution. Also, it doesn't do live-range splitting, so it will only merge two variables if they can be merged over their entire lifetimes (and I suspect that's the problem here)

It also turns out that I can't do an optimal job at the source level.

• every function uses a hidden first argument as "self". That argument is often not used (or only used early on), so becomes available for sharing; but there's no way to do that at the source level.
• Similarly, every catch block creates a new variable, whether you use it or not, and they're all distinct; and there's nothing I can do at the source level.

So at the moment, I'm working on redoing this optimization in the post-build optimizer, together with full live-range splitting, which should certainly solve that issue (but you won't be able to see it without inspecting the byte code).

flocsy🤠 said:

you can see how ly is used in the loop. it's read twice and increased once

Thanks - just calling that with random values was enough to repro the bug. I'm going to try creduce on it, just to see if I can make it work...

It was a bit of a pain writing the creduce tests, because it would tend to remove all uses of ly, so that the if could be legitimately removed, but I eventually got something that produced this:

  var mGaugeY as Number = 42;
  hidden function drawHrGaugeHand(dc as Dc, triangleHeight as Number, gaugeOnTop as Boolean, gaugeHeight1 as Number, hrx as Number) as Void {
    var y = mGaugeY;
    dc.drawLine(hrx, y, hrx, y + gaugeHeight1);
    var ly = y;
    if (gaugeOnTop) {
      ly += gaugeHeight1;
    }
    var i = 0;
    do {
      dc.drawLine(hrx - i, ly, hrx + i, ly);
    } while (i <= triangleHeight);
  }

It had actually removed all the parameters, and the mGaugeY variable; I manually added them back just to make sure that the issue wasn't caused by missing variables. Overall I think it worked quite well...

Anyway, I'll debug...