icicle-lang / icicle-ambiata

A streaming query language.
BSD 3-Clause "New" or "Revised" License
57 stars 11 forks source link

Flatten/Constructor: don't duplicate #653

Closed amosr closed 6 years ago

amosr commented 6 years ago

A while ago I noticed some duplicate work in the flattened avalanche:

feature salary ~> fold y = 1 : 1 ~> y / 0

Avalanche
----------
y-conv-4 =r acc-y-conv-4
anf-1 = div# y-conv-4 0.0
anf-0 = doubleIsValid# anf-1
flat-0 =i Left ExceptNotAnError : Sum Error Double

if (anf-0)
{
    flat-0 =w right# anf-1
}

Flattened, simplified
--------------------
y-conv-4 =r acc-y-conv-4
anf-1 = div# y-conv-4 0.0
anf-0 = doubleIsValid# anf-1
flat-0-simpflat-0 =i ExceptNotAnError : Error
flat-0-simpflat-1 =i 0.0 : Double

if (anf-0)
{
    flat-0-simpflat-0 =w ExceptNotAnError
    flat-0-simpflat-1 =w div# y-conv-4 0.0
}

The div# gets duplicated into two places! The constructor transform is putting every let-binding it sees in the environment, and when trying to melt right# anf-0 looks up the arguments in the environment, so it ends up inlining the definition of anf-0.

I think it only needs to inline things which are going to cause a melt/unmelt transform to fire: so values, melt prims, unmelt prims, as well as variables which might point to any of those. Is that sufficient?

amosr commented 6 years ago

A lot of the duplicated work is just floating point operations. So it's not going to make a big difference in runtime. Perhaps even slightly slower, depending on register pressure. But making the environment smaller should speed up constructor a bit, as well as producing smaller output code.

amosr commented 6 years ago

! @jystic @tranma forgot jury

tranma commented 6 years ago

ok, this made sense. 👍