Big update to prettier-extension-monkeyc

Yeah, I was confused. I thought I'm checking the returned value for being null.

Still no.

Original: code: 11097, data: 4141

const ZONE_COLORS_ARR as Array<Graphics.ColorType> = [Graphics.COLOR_LT_GRAY, Graphics.COLOR_BLUE, Graphics.COLOR_GREEN,
    Graphics.COLOR_YELLOW, Graphics.COLOR_RED, Graphics.COLOR_DK_RED] as Array<Graphics.ColorType>;

(:inline)
hidden function getZoneColor(backgroundColor as Graphics.ColorType) as Graphics.ColorType {
    var heartRateZone = mHeartRateZone;
    return mShowZoneColor && 1 <= heartRateZone && heartRateZone <= 6 ? ZONE_COLORS_ARR[heartRateZone - 1] : backgroundColor;
}

public onUpdate(dc) {
    var zoneColor = getZoneColor(backgroundColor);
    ...
    for (var z = one; z <= 5; z++) {
        dc.setColor(ZONE_COLORS_ARR[z - 1], backgroundColor);
    }
}

6bit code: 11127, data: 4133 = c:+30, d:-8 = +22

If I switch the order of ZONE_COLORS5 and rgb6 then: 11130, 4133 = +25

If I remove :inline from color6: code: 11092 data: 4151 = +5

private const ZONE_COLORS6 as Long =
    // note the trick: I skip 0 so I don't need (zone - 1) in 2 places in my code
    (rgb6(Graphics.COLOR_LT_GRAY) << 6) +
    (rgb6(Graphics.COLOR_BLUE) << 12) +
    (rgb6(Graphics.COLOR_GREEN) << 18) +
    (rgb6(Graphics.COLOR_YELLOW) << 24) +
    ((0L + rgb6(Graphics.COLOR_RED)) << 30) +
    ((0L + rgb6(Graphics.COLOR_DK_RED)) << 36);
// (:inline)
hidden function rgb6(color as ColorType) as Number {
    return (
        ((color & 0xC00000) >> 18) +
        ((color & 0xC000) >> 12) +
        ((color & 0xC0) >> 6)
    );
}
(:inline)
function color6(index as Number) as ColorType {
    // extract the 6-bit field based on the zone
    var field = (ZONE_COLORS6 >> (index * 6)).toNumber();
    // convert the 6-bit color back to 24-bits
    return
        (field & 0x03) * 0x55 +
        (field & 0x0C) * 0x1540 +
        (field & 0x30) * 0x55000;
}
(:inline)
function color(zone as Number) as ColorType {
    return color6(zone);
}

(:inline)
hidden function getZoneColor(backgroundColor as Graphics.ColorType) as Graphics.ColorType {
    var heartRateZone = mHeartRateZone;
    var heartRateZoneColor = color(heartRateZone); // here I also need to have an additional local variable because of the restrictions of the optimizer: While inlining color: This function can only be inlined in statement, assignment, if or return contexts
    return mShowZoneColor && 0 < heartRateZone && heartRateZone < 7 ? heartRateZoneColor : backgroundColor;
}

public onUpdate(dc) {
    var zoneColor = getZoneColor(backgroundColor);
    ...
    for (var z = one; z <= 5; z++) {
        var zoneColor = color(z);
        dc.setColor(zoneColor, backgroundColor);
    }
}

5bit code: 11135, data: 4133 = +30

If I switch the order of ZONE_COLORS5 and rgb5 then: 11134, 4133 = +29

If I remove :inline from color5: code: 11103 data: 4151 = +16

private const ZONE_COLORS5 as Number =
    (rgb5(Graphics.COLOR_LT_GRAY) << 0) +
    (rgb5(Graphics.COLOR_BLUE) << 5) +
    (rgb5(Graphics.COLOR_GREEN) << 10) +
    (rgb5(Graphics.COLOR_YELLOW) << 15) +
    (rgb5(Graphics.COLOR_RED) << 20) +
    (rgb5(Graphics.COLOR_DK_RED) << 25);
// (:inline)
hidden function rgb5(color as ColorType) as Number {
    return (
        ((color & 0xC00000) >> 19) +
        ((color & 0x4000) >> 12) +
        ((color & 0xC0) >> 6)
    );
}
(:inline)
function color5(zone as Number) as ColorType {
    // extract the 5-bit field based on the zone
    var field = ZONE_COLORS5 >> (zone * 5);
    // convert the 5-bit color back to 24-bits
    return
        (field & 0x03) * 0x55 +
        ((field & 0x04) + (~zone & 4) * 2) * 0x1540 +
        (field & 0x18) * 0xAA000;
}
(:inline)
function color(zone as Number) as ColorType {
    return color5(zone - 1); // here I have to have -1 in 2 places in my code
}

(:inline)
hidden function getZoneColor(backgroundColor as Graphics.ColorType) as Graphics.ColorType {
    var heartRateZone = mHeartRateZone;
    var heartRateZoneColor = color(heartRateZone); // here I also need to have an additional local variable because of the restrictions of the optimizer: While inlining color: This function can only be inlined in statement, assignment, if or return contexts
    return mShowZoneColor && 0 < heartRateZone && heartRateZone < 7 ? heartRateZoneColor : backgroundColor;
}

public onUpdate(dc) {
    var zoneColor = getZoneColor(backgroundColor);
    ...
    for (var z = one; z <= 5; z++) {
        var zoneColor = color(z);
        dc.setColor(zoneColor, backgroundColor);
    }
}

And I think both rgb6 and rgb5 will be slightly better when the bugs are fixed, but probably will still not be as good as the original.

bug1: changing the order of the array and the rgb method shouldn't matter

bug2: it should recognize that the const is used twice in both cases (no matter the order)

feature, not sure if possible: if color(z) could be used "in place" then 2 less local variables would be necessary 

flocsy🤠 said:

Still no.

Ok - interesting. I should go back and re-measure with my app.

I first did this before I wrote the optimizer - so one issue was that filling the array with 5 "Graphics.Color" values took a lot of code. I remember realizing then that I could save a lot by using the actual numbers instead; but still, switching from that to the rgb6 version (hand coded of course) saved a couple of hundred bytes if I remember correctly - and overall it was about a half a k.

But now both my optimizer and Garmin's will get rid of the Graphics.Color values for you, so I knew the win would be much smaller; but I still expected something. Maybe their optimizer is doing more about initializing "constant" arrays efficiently.

One thing to note though is that you're not accounting for heap memory.

I seem to remember a 5 element array uses 45 heap bytes; so when you take that into account, both versions are saving something - but nowhere near what I was expecting.

One other thing you could try though... since you're looping over the colors, you could probably make color extraction a bit more efficient - something like:

(:inline)
function colorFromField(field as Number) as Number {
    // convert the 6-bit color back to 24-bits
    return
        (field & 0x03) * 0x55 +
        (field & 0x0C) * 0x1540 +
        (field & 0x30) * 0x55000;
}

and then in your code:

    var field = ZONE_COLORS6;
    for (var i = 1; i < 6; i++) {
      var color = colorFromField(field);
      field >>= 6;
      ...
    }

Well no.. It made things worst in my code. And this leads us back to an older discussion. The problem is that it's very chaotic. For example I tried now to apply your last idea (passing field instead of zone) to both the 6 and 5 bit versions. I got c:11117,d:4133. Then I realized I forgot a "-1" in one place so I added it, but then I got: c:11147,d:4151, because it made color5 not inlinable... So I had to add var z_1=z-1; and I got: c:11124,d:4133

I'll give it another try after you release the next version (assuming that it'll take care of the 2 bugs)

Also: can you add the code and data size that you print in the log also to build-info.json? It'll make comparsions easier, because I need to compare 5 versions (one from each "class" of devices)

flocsy🤠 said:

I'll give it another try after you release the next version

v2.0.47 is out.

• Fixes the inline ordering issue which could prevent fully optimizing the resulting code
• Fixes the cost of Long and Double constants, so that pre only requires 2 uses to create a local
• Does a better job of recording the original name of variables when the variable minimizer combines multiple variables into one. In particular if a local was renamed after inlining, it should now report the original name. Still records pre_* names rather than original name/values though.
• Fixes some other obscure issues (didn't affect any of the open source code I compile, or my projects, but I wrote some test cases that didn't work correctly after optimization).
• Adds a Hover Provider to the language support features so you get info about the symbol under the mouse.

flocsy🤠 said:

I still see differences when I switch the order of ZONE_COLORS6 and rgb6

You're right. I was sure I'd fixed that. But at least the initializer is now a literal.

For me the code size is exactly the same; which is why I didn't notice. I guess Garmin's optimizer is getting rid of the constant anyway these days...

flocsy🤠 said:

I noticed that the one that eliminated the const has pre_2251505580672l, pre_ZONE_COLORS would make more sense

Probably not going to happen... it would certainly be possible to keep track of the origin of a constant (at least, so long as there's only one), and then name the pre variables; but it would be tricky, probably unreliable, and just doesn't seem worth the effort (since there are plenty of things to do that can actually help). And if I ever figure out how to generate a binary directly, it would be moot; there wouldn't be any source code to look at.

There are so many versions I tried (rgb6 with 1 color(), rgb6 with color+zone2field, the same with rgb5) and not in all of them I saw the difference. This is part of the chaoticness

As for the direct binary: well, I don't know. Currently it still benefits here and there from what the Garmin compiler does. And even if you would be able to generate binary I guess there should be a setting to generate the source, because without that there would be no way we could report things...

I noticed another possibility to optimize out a local variable that is only used once:

function onUpdate(dc) {
  var width = dc.getWidth();
  var height = dc.getHeight();
  var cond = isCond();
  var isWide = height * 2.8 < width;
  var foo = width * (isWide && cond ? FOO :BAR) / 100;
  // ... width, height, foo are used, but isWide is not used anywhere else, so it could be inlined and the local variable removed.
}

function a() {
  var zero = 0;
  var s = mSensor;
  var m = s.d.e == zero ? 1 : -1;
  mX.foo(toV(s.d.x, zero, m));
}

(:inline)
function toV(value as Number?, nullValue as Number, multiplier as Number) as Number {
  return value == null ? nullValue : value * multiplier;
}

Foo.mc:134:13: While inlining toV: This function can only be inlined in statement, assignment, if or return contexts

Why this can't be inlined? I thought that these simple 1-liners do well in inlining. This looks literally like "copy&paste" :) (I know it's not that simple, but still)