pigpigyyy / Yuescript

A Moonscript dialect compiles to Lua.
http://yuescript.org
MIT License
439 stars 37 forks source link

Silent generation of anonymous functions #78

Open megagrump opened 2 years ago

megagrump commented 2 years ago

I have this function:

export push = ->
    matrix = matrixPool::pop!?::copy(globalTransform) or Matrix.copy(globalTransform)
    transformStack::push(matrix)
    nil

As I'm always paranoid about generating unnecessary garbage, I noticed the memory counter going up when calling this function.

It compiles to:

push = function() -- 48
    local matrix = (function() -- 49
        local _obj_0 = matrixPool:pop() -- 49
        if _obj_0 ~= nil then -- 49
            return _obj_0:copy(globalTransform) -- 49
        end -- 49
        return nil -- 49
    end)() or Matrix.copy(globalTransform) -- 49
    transformStack:push(matrix) -- 51
    return nil -- 52
end -- 48

So the culprit for the garbage is the anonymous function that was generated for this code.

I changed the code to a more conservative form:

export push = ->
    matrix = matrixPool::pop! or Matrix!
    matrix::copy(globalTransform)
    transformStack::push(matrix)
    nil

which compiles to:

push = function() -- 48
    local matrix = matrixPool:pop() or Matrix() -- 49
    matrix:copy(globalTransform) -- 50
    transformStack:push(matrix) -- 51
    return nil -- 52
end -- 48

and the problem went away.

This makes me wary of the existence operator, because I want to write fast code that generates no more garbage than necessary.

I think it would be prudent if there was an option to emit a warning when an abstraction leads to a potentially expensive construction like this. Otherwise it is necessary to proof-read the generated code if you're performance-aware but don't know exactly how an abstraction will be compiled.

pigpigyyy commented 2 years ago

There is a hidden rule to avoid creating anonymous function by assigning some complicated expression to a temp variable.

export push = ->
  mat = matrixPool::pop!?::copy(globalTransform) -- assigning expression to an extra local variable
  matrix = mat or Matrix.copy(globalTransform)
  transformStack::push(matrix)
  nil

compiles to:

local _module_0 = { } -- 1
local push -- 1
push = function() -- 1
  local mat -- 2
  local _obj_0 = matrixPool:pop() -- 2
  if _obj_0 ~= nil then -- 2
    mat = _obj_0:copy(globalTransform) -- 2
  end -- 2
  local matrix = mat or Matrix.copy(globalTransform) -- 3
  transformStack:push(matrix) -- 4
  return nil -- 5
end -- 1
_module_0["push"] = push -- 5
return _module_0 -- 5

Or you can try the nil coalescing operator which is turning expressions into multiple lines of assignment.

export push = ->
  matrix = matrixPool::pop!?::copy(globalTransform) ?? Matrix.copy globalTransform
  transformStack::push matrix
  nil
local _module_0 = { } -- 1
local push -- 1
push = function() -- 1
  local matrix -- 2
  do -- 2
    local _exp_0 -- 2
    local _obj_0 = matrixPool:pop() -- 2
    if _obj_0 ~= nil then -- 2
      _exp_0 = _obj_0:copy(globalTransform) -- 2
    end -- 2
    if _exp_0 ~= nil then -- 2
      matrix = _exp_0 -- 2
    else -- 2
      matrix = Matrix.copy(globalTransform) -- 2
    end -- 2
  end -- 2
  transformStack:push(matrix) -- 3
  return nil -- 4
end -- 1
_module_0["push"] = push -- 4
return _module_0 -- 4

And I agree with your idea of adding an option to emit warning when code compiles to unintended anonymous functions. Maybe a lint option could be added.