code.pyret.org icon indicating copy to clipboard operation
code.pyret.org copied to clipboard

_vertices array for images considered harmful

Open blerner opened this issue 4 years ago • 1 comments

Related to https://github.com/brownplt/code.pyret.org/issues/361, the _vertices array on images is a source of quadratic or worse memory bloat. Worse, the following code causes Chrome to crash CPO with a stack error, even though there's no actual stack unsafety present:

include image

fun fractal(depth):
  if depth == 0:
    rectangle(2, 2, "solid", "red")
  else:
    f = fractal(depth - 1)
    above(f, beside(f, f))
  end
end

fractal(9) # Raise this to 10 to crash

I'm being nice by memoizing the nested fractal calls, but the outermost above image results in an image whose _vertices array contains nearly 79K vertices (4 * 3^9), from concatenating all the vertices of each rectangle over and over and over again, even though there are only 19 images allocated in this computation (1 RectangleImage and 18 OverlayImages). If I didn't memoize, then we'd also have 79K RectangleImages and ~19K OverlayImages to deal with, further raising the memory pressure.

I don't think we need the _vertices array, and since they're only used for bounding-box computations which we can (and probably should) compute differently anyway, I think we should eliminate this.

blerner avatar Dec 24 '20 23:12 blerner

I finally had a chance to dig through the WeScheme codebase (which is more familiar to me). You're right -- we do not need this array. At one point I had designs on it being a part of some reconciliation algorithm akin to what Robby does in Racket, but it only wound up serving as an over-engineered solution to a much simpler problem (computing bounding boxes). I say kill it. Go with god.

schanzer avatar Dec 27 '20 19:12 schanzer