Cl/remove some event streams from block

[cowboyd]

Motivation

When using Effection in the browser under heavy load, performance was unacceptable.

Approach

After profiling in a production application, and through a set of benchmarks, the performance and memory profiler revealed that the biggest slow down was caused by frequent pauses for major garbage collections.

It turned out that the biggest issue here was with memory consumption with each Frame. Unfortunately, the frame was a very, very, very heavyweight object that included:

• 5 EventStream objects (two for itself, and three for its Block)
• 2 Futures
• 1 AbortController
• many, many intermediate generators.

For starters, this change completely and totally unrolls the Frame into a single computation. All of the Block code has been "inlined" into the Frame which completely removes the need for any communication queues between a Frame and its Block. This eliminated three event streams.

Next, it became clear that the queue of "thunks" that tracked instructions to be run did not to be async in any way shape or form, so that EventStream was replaced by an array.

It turns out that AbortController is a really heavyweight object, and expensive for the runtime to create, so it is replaced by a simple JS object that implements the signal.aborted flag (we may want to consider putting that flag on Frame and get rid of the need for this object altogether).

The Futures which were created eagerly with each task have been changed to be lazy. You won't get any Future created at all unless you call Task.then() or Task.halt().then(). This has the benefit that Task.halt() is now a stateless operation, just like every other operation in the system. It will not do anything until it is either yield*ed or awaited.

Finally, an effort was made to remove all necessary generators along the hot path. This was done by substituting in simple functions wherever possible rather than creating intermediate generators whose only purpose was to yield to other generators. Part of this introduces as shiftSync as a complement to shift which takes a simple function to serve as a simple computation of the shift.

The result is, in many ways, more legible, because the entire Frame computation, while long, is a single generator function that can be read from top to bottom, and very clearly sequences what is happening.

A "performance" test was added to test Effection performance relative to the base system in order to make sure that we don't inadvertently introduce something that tanks performance.

Before

$ deno run benchmark.ts
{ measure: 958 }

After

$ deno run benchmark.ts
{ measure: 198 }

Alternate Designs

• make the perf tests part of a separate test suite deno task perf because they necessarily take a long time and we don't want them interfering with speedy development.

[neurosnap]

Overall much easier to reason about, nice work!

I would argue that we should move as much code outside of DC as possible in this function. The reason is because there's only a couple areas that actually require DC in order to work properly -- and opens us up for further perf optimizations if there's a clear separation. Whereas there's quite a bit of preamble here that doesn't require DC at all.

[neurosnap]

This is a red flag to me and something I find in general very confusing to reason about. We are passing self to createTask but we are still defining the shape of self at this point.

If createTask needs the frame it seems like createTask could be merged into createFrame instead of it being passed in, especially because the task needs an event for when the frame is completed/halted. Thoughts?

[cowboyd]

I agree. The main reason to keep createTask() separate was to make unrolling the Future logic tractable. Now that this is done, it makes sense to attempt yet another level of unrolling.

[cowboyd]

I tried unrolling it without much success. I don't think we're still determining the shape of Frame at this point. This was part of the work to make the operations and promises and all of the moving bits of task lazy so that they never get created unless you are using them explicitly. I disentangled the enter() method from the getTask(). This means that now you can enter a frame without actually creating a task at all. Which I think is much better.

Finally, for the reference to self. It should now be written in a way where we could use this and class syntax if we wanted to.

Let me know what you think.

[neurosnap]

Why do we need to return the frame here when we must already have it in scope when calling these methods?

[cowboyd]

We're using the Frame as Computation<FrameResult> in this context, because the method signature is destroy(): Computation<FrameResult>

[neurosnap]

I guess my question is: why? Couldn't we:

frame.destroy();
yield* frame;

[cowboyd]

I think it has to do with the history of destroy() being a computation in its own right.

frame.destroy() used to not do anything until you evaluated it. However, I made it a synchronous method to try and eliminate an intermediate generator.

effection/lib/run/frame.ts

Lines 69 to 79 in e72b9ac

	*crash(error: Error) {
	setTeardown(Err(error));
	return yield* frame;
	},
	*destroy() {
	setTeardown(Ok(void 0));
	return yield* frame;
	},
	*[Symbol.iterator]() {
	return yield* results;
	},

In a more general sense, I like methods that tell you when they are done rather than "fire and forget" followed by join at certain point.

yield* frame.destroy();

When destruction is complete is ultimately the business of the frame, not the caller, so having it signal that destruction is fully settled when the computation returns feels like a good api.

[neurosnap]

Makes sense

[neurosnap]

Here you are referencing a frame before it has been created -- this is confusing to reason about. Further, this function is used inside frame.crash and frame.destroy. This is another example of a circular reference that makes me nervous.

[cowboyd]

All of the circular references were removed.

[neurosnap]

I had just one more comment and that's it! Nice work, definitely a huge improvement in terms of readability and performance.