Big update to prettier-extension-monkeyc

flocsy said:

It also depends on how many times you call the function, but the smaller the code in the function the inclining gains more

Right, but it can be surprising what's bigger, and what's smaller. Which was my point with your original example.

Given something like:

function getConfig(x) {
  return Application.Properties.getProperty(x);
}

function foo() {
    x = getConfig("x");
    y = getConfig("y");
    z = getConfig("z");
}

it's tempting to think that inlining is a good idea... you're just replacing one call with another, and you get rid of the original function. But in fact, each call to Application.Properties.getProperty takes several more bytes than a call to getConfig. I just tested, and my guess upthread was in fact wrong. With just three callsites it *is* better to inline. But with four, it's almost flat, and five it's a definite win to keep getConfig.

And maybe thats the answer to inline_foo. Suppose your fr235 app is already a cut down version of the real thing, because you had to trim features to get it to fit. So for fr235 you have 3 calls to getConfig, and for new devices you have 10. Without the inline_foo feature, when you export a .iq file, either getConfig gets inlined for every device, or it doesn't get inlined for every device. But by using :inline_smallmem, you can have it inlined on fr235 and similar (where its a memory saving) but not inlined on big memory devices (where it's not).

flocsy said:

but also I think your annotation is counterintuitive because it works the opposite way

I kind of agree, and in fact I tried it both ways, found both confusing, but less so the way I ended up doing it.

One thing to note here is that a (:foo) annotation can be used to exclude things in normal projects, but to *include* things from barrel projects (via barrelName.annotations = foo). So its already pretty mixed up what an annotation means...

FlowState said:

Not to contradict anything else written in this thread

Yes. It's definitely true that there are functions that are always a win to inline. Its just that identifying them isn't always easy.

Ok, but if that is the case then why don't you call it that way: (:inline_upto3) or something like that. That's also useful because then later when you use it it a)still minimizes the size by not inlining if you added a 4th call, b)when you add the 4th call it reminds you to check things

flocsy said:

why don't you call it that way: (:inline_upto3)

Because then thats the only thing you can do with it. As it stands, you (the writer of the app) can choose to inline based on number of times its called, or (as I said earlier), based the fact that inlining will be more performant, even if its larger.

So eg I might have 100 calls to

function get() {
    return Foo.Bar.Baz.buz;
}

I *know* its going to be more performant to just stick Foo.Bar.Baz.buz at each call site (I mean, the function does exactly the same work, but we *also* have the call overhead). So on my 128k-for-a-datafield fenix5xplus, I don't care about the extra 500 bytes it takes. But on my fr235, I care a lot...

But ultimately, if *you* don't have a use for it, *you* don't have to use it!

flocsy said:

Here's another place, setConfig is uses 3 times, for some reason it wasn't inlined either

I've just released 2.0.14 with support for inlining most functions, so long as the return value isn't used.

The only real constraint on the function itself is that it can have at most one return statement, and if it has one, it must be the last statement in the outermost block. Its ok for the return statement (if there is one) to return a value - the constraint is on the caller not to *use* the value.

Note that if the function's return value *is* used, the original constraints still apply.

markw65 said:

I've just released 2.0.14

That one had an inliner bug. Luckily, if the bug is triggered, the optimized code fails to compile, so it doesn't just silently generate bad code. Its fixed in 2.0.15.

Latest version v2.0.16 includes support for inlining when the call is the rhs of an assignment, or the argument to "return".

For assignments, the lhs has to be either a variable, or a member-expression (ie either "x = foo();" or "x.y.z = foo();"). For "update" assignments (ie +=, -=, *= etc), the lhs has to be a local variable; otherwise order of evaluation issues can be a problem. Also, the function to be inlined must contain a single return statement, which must be the (lexically) last statement in the body of the function.

For "return foo();" the only constraint on foo is that the (lexically) last statement in the body of the function has to be a return statement (but there can be other return statements in the function).

I also added diagnostics for when a function labelled (:inline) doesn't actually get inlined.

I found a bug in 2.0.16: the inlined function isn't removed!

I tested this code:

    (:inline)
    hidden function handleSettingUpdateCommon() as Void {
        if (mSensor != null) {
            setAntName(antId2Name(mSensor.deviceCfg.deviceNumber));
        }
    }
    (:memory16K)
    public function handleSettingUpdate() as Void {
        handleSettingUpdateCommon();
    }

with and without the (:inline), and I see that the code is bigger when inlined, which makes no sense as this is the only place it's being used, so I expected the optimized code to look like:

//    (:inline)
//    hidden function handleSettingUpdateCommon() as Void {
//        if (mSensor != null) {
//            setAntName(antId2Name(mSensor.deviceCfg.deviceNumber));
//        }
//    }
    (:memory16K)
    public function handleSettingUpdate() as Void {
//        handleSettingUpdateCommon();
        if (mSensor != null) {
            setAntName(antId2Name(mSensor.deviceCfg.deviceNumber));
        }
    }

However the actual optimized code looks like this:

    (:inline)
    hidden function handleSettingUpdateCommon() as Void {
        if (mSensor != null) {
            setAntName(antId2Name(mSensor.deviceCfg.deviceNumber));
        }
    }
    (:memory16K)
    public function handleSettingUpdate() as Void {
        {
            if (mSensor != null) {
                setAntName(antId2Name(mSensor.deviceCfg.deviceNumber));
            }
        }
    }

flocsy said:

I found a bug in 2.0.16: the inlined function isn't removed!

Yes I see what's going wrong, and it affects anything that doesn't get inlined as an expression. When it fails to inline it as an expression, it marks the function as having been called (which implies it can't be deleted), but then shortly after, it successfully inlines it as a statement. I just need to do a separate pass at the end to determine which functions are actually called. I'll hopefully get a fix out tomorrow.