community icon indicating copy to clipboard operation
community copied to clipboard

New app: Amazing

Open dinosaursrarr opened this issue 1 year ago • 4 comments

Draws mazes to fill the space on the screen. Lovely screensaver vibes.

Has 10 different algorithms. Default behaviour is to pick on at random.

Have been working on this for a little while. Progress greatly slowed by falling off my bike and breaking my wrist :(

dinosaursrarr avatar Jul 23 '22 21:07 dinosaursrarr

Looks like tests are failing due to 404 downloading one of the dependencies. Don't think it's a problem with my changes. make lint ran fine locally.

dinosaursrarr avatar Jul 23 '22 21:07 dinosaursrarr

Hey @dinosaursrarr! So sorry about your bike accident 😩 . This is really pretty. I'm a bit nervous about how expensive it is to run though:

time pixlet render apps/amazing/amazing.star
pixlet render apps/amazing/amazing.star  42.88s user 2.78s system 319% cpu 14.301 total

We just had to disable the Life app for similar issues. Any idea if we can make this faster to render, or potentially limit the designs to ones that are a bit less intensive?

betterengineering avatar Aug 04 '22 16:08 betterengineering

@betterengineering I can definitely take a look. I suspect it's because I'm rendering more frames for a smoother animation.

What's a good benchmark render time to aim for?

dinosaursrarr avatar Aug 04 '22 18:08 dinosaursrarr

Let me put together a proper unit test for performance - it will help take the guess work out of it. Honestly, it should be able to render several times in a second, but we're finding there are several rather slow apps. At the worst case, it shouldn't take longer then 5 seconds to render.

I've just pushed a new commit that should improve things. The time spent in Starlark is now negligible (vs seconds). It looks like the bigger issues are on the pixlet render side, which seems to add several seconds. I've at least made it easier to tune performance by changing a single constant. Otherwise, it seems a bit out of scope for me. https://github.com/tidbyt/community/pull/560/commits/7ce3b15547ab91be251312376aa5e40367670b1e

dinosaursrarr avatar Aug 08 '22 20:08 dinosaursrarr

We're automatically closing this issue because it hasn't had any activity in 30 days. If that seems like a mistake, please feel free to re-open. Thanks!

stale[bot] avatar Sep 09 '22 03:09 stale[bot]

I don't think this is stale. As far as I understood, it was waiting on @betterengineering 's plan to add unit tests for render performance. (It says you requested changes, and maybe I'm being dense, but I can't see any specific changes you were asking for).

I also don't seem to be able to reopen this unless I'm missing something?

dinosaursrarr avatar Sep 09 '22 13:09 dinosaursrarr

Hey there! Sorry the bot was too aggressive 😭 . The change required is this app has to render more quickly or we can't really support it. I'd like to put together a unit test to make it more clear to folks trying to submit apps, but it's not a requirement for this change

betterengineering avatar Sep 14 '22 21:09 betterengineering

Hey @dinosaursrarr! I'll merge this today and start testing it out in production. With a bit of luck we can get it out. The rendering seems to be fast enough right now. The remaining problem is that with completely random graphics for each execution, we'll end up with a very large number of distinct graphics, which in turn messes up some pretty critical caching in our backend.

I recently merged https://github.com/tidbyt/pixlet/pull/398 which keeps RNG state for each app execution. That combined with seeding the RNG with time.now().unix / N for some reasonable choice of N should keep things "random enough" without wrecking the cache.

So yeah, I'll test this out a bit, possibly cut a PR for the seed() call if needed.

matslina avatar Sep 22 '22 16:09 matslina