Yuescript icon indicating copy to clipboard operation
Yuescript copied to clipboard

Silent generation of anonymous functions

Open megagrump opened this issue 3 years ago • 1 comments

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.

megagrump avatar Feb 12 '22 19:02 megagrump

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.

pigpigyyy avatar Feb 14 '22 01:02 pigpigyyy