3.0.0-preview4, batch rendering, async, server-side

[WilStead]

• Implements interop call batching (resolves Batch rendering #13)
• Switches to async model
• Adds support for server-side Razor Components (addresses CreateCanvas2d() Fails in Server-Side Blazor Apps #12)
• Updates to 3.0.0-preview4 (resolves Blazor 0.8.0 Upgrade #22)

Sorry for the massive PR that touches so much code, but it seemed necessary to do so in order to fully address the big issues. I made an attempt to implement the changes you've discussed in #12 and #13. I'll provide a rundown of the major changes and my reasoning:

In order to support Razor Components server-side, I updated the dependency to the latest version (0.9.0) and changed the base class of BECanvasComponent to ComponentBase (which supports both Blazor client-side and Razor Components server-side). The required updating the project to the .NET Core 3.0 preview. I altered the CI build to use that SDK.

Since the static, synchronous JSRuntime.Current has been deprecated in favor of injecting IJSRuntime and making async calls (the only option for server-side), I refactored the entire library to adopt an async approach. In most cases I left the synchronous API in place, and those methods now wrap the async calls, but are marked as Obsolete with a message recommending that users switch to the async version. In the case of property setters, wrapped async calls can result in unresponsive UI, so I opted instead for a breaking change: read-only properties with async property setter methods.

I implemented a simple call batching solution. "Fire-and-forget" calls which do not return values (both property setters and method calls) execute immediately if there are no pending interop calls, and otherwise are stored in a list until the latest interop call completes. Access is synchronized with a SemaphoreSlim to avoid threading issues.

Calls which return a value are never batched: it created a negative performance impact, instead of an improvement, when I attempted to implement a callback mechanism that cached incoming calls, awaited results, matched them to initial callers, then dispatched callback messages.

Server-side code and WebGL don't work well together. Because of the server-side rendering mechanism, the last draw operation overrides all previous operations. I implemented a solution which allows users to explicitly group calls into batches, using BeginBatchAsync and EndBatchAsync API calls, analogous to BeginUpdate and EndUpdate desktop UI APIs. I'm not satisfied that this solution will resolve all the potential problems, and I admit that I didn't run any extensive WebGL tests (animation, etc), I just made sure that the basic tests already present were working.

Speaking of tests, I added a second test project for server-side Razor Components. I also renamed the existing test project to reflect the fact that it's the client-side version (and updated it to v0.9.0).

Server-side Razor Components currently has a bug (dotnet/aspnetcore#6349) which I'm sure you're aware of, that prevents component libraries from loading static files. It seems based on the status of that item that they're close to releasing a fix, but in the meantime I added steps to the Typescript project's MSBuild which copies the compiled .js file directly to a dist folder in the server-side test project's wwwroot, and also places a copy at the root of the solution (for easy user downloading).

I updated the README to reflect all the changes.

I also incidentally made a change that I believe @jorolf suggested earlier: I added rules to .editorconfig to reflect the underscore naming convention in use throughout most of the library, and updated a few variable names that didn't already adhere to it.

I realize that what I've attempted here may be way off-base from what you have in mind. If you prefer not to adopt this massive PR, feel free to simply copy any bits of code you find useful.

[galvesribeiro]

Thanks for the PR. I'll have a look over the week and get back to you as soon as possible.

[michaelp]

any updates on this PR?
What is the ETA?

[jorolf]

Do you have any benchmarks regarding the speed difference between calling the methods directly and making them async?

[jorolf]

I'm not sure how easy it would be but what if you make this return an IDisposable which calls EndBatchAsync on disposal. The syntax would look something like this:

using (this._context.BeginBatchAsync()) {
  await this._context.ClearAsync(BufferBits.COLOR_BUFFER_BIT);
  await this._context.DrawArraysAsync(Primitive.TRIANGLES, 0, 3);
}

However I'm not sure if this is going to work because of all the async stuff

[WilStead]

I do like the fact that this paradigm prevents accidentally beginning a batch and forgetting to end it. You're correct that the async "end batch" call complicates things, though. I could probably implement it using either a 'fire-and-forget' task, or a blocking call like the ones that are currently used in the old API wrappers. Either solution has a few major drawbacks, though, and I'm not sure they would be good solutions.

A fire-and-forget async call in a Dispose method would mean that the operation isn't necessarily complete after control exits the using block, and there would be no way to await on it. That would prevent any follow-up code that depends on the operations inside the block being actually finished. Probably not feasible from a usability perspective.

A blocking call avoids that issue, but it has another problem: what if the EndBatchAsync call fails? Dispose isn't supposed to fail, and using statements tend to swallow exceptions in their try block if the finally block also throws. In the worst case the call might block indefinitely, leaving the thread blocked in the using statement's implied finally clause.

The IAsyncDisposable proposal aims to resolve these concerns and some others. If it makes it into c# 8.0 we could take advantage of it. That proposal has been around a while, though, and I haven't seen any official word that it's going to arrive soon.

[WilStead]

@jorolf I don't have any benchmarks, no. I assume that using the synchronous calls would be slightly faster. The smart batching system should keep the difference small in client-side situations where the calls are able to execute almost synchronously as it is.

I decided to move entirely away from using the synchronous JS interop instance based on Microsoft's guidance: "We recommend that most JavaScript interop libraries use the async APIs to ensure that the libraries are available in all scenarios, client-side or server-side." They do say "most," though, and a library like this one may be a good candidate for making an exception.

There do seem to be ways to detect at runtime whether an app is running client-side or server-side. We could dynamically detect when the code is running client-side, and call the synchronous JS interop methods on the static instance when possible.

Or we could just leave the old API calls synchronous and leave it up to callers to use them appropriately. We would provide user guidance in the README and the code comments that explains that the async calls work in any situation, but the synchronous calls fail in server-side code. It would be the library consumer's responsibility to call the correct APIs based on their situation, or implement their own runtime detection strategy if they wanted to handle both cases.

I think any of those scenarios would be fine, really.

[pconfig]

Any update on merging and packaging this?

[matteo-prosperi]

I just started playing around with Blazor, so my understanding is still very superficial. Sorry If what I describe below is not as in depth as it could be.

I tried this pull request out because I need a version of BlazorExtensions/Canvas that works with the Blazor 0.9. In my test application I have a for loop in C# calling 14400 times FillRectAsync on a Canvas2DContext.

When I run my code without BeginBatchAsync();, it takes 22 seconds to complete the drawing.
When I run my code with BeginBatchAsync();, it takes 12 seconds.

I also tried using a StringBuilder to concatenate a single string with the 14400 javascript fillRect calls. I then passed the string to a single InvokeAsync call, having a javascript function execute all the commands by running a single eval call. With this "crude" approach, the whole drawing takes only 2 seconds to complete.

These results seem to point to callBatch having space for significant optimization. They also show that batching calls is likely a very good idea and necessary for complex drawings.

[WilStead]

@matteo-prosperi Your StringBuilder method certainly seems to perform much better. I wonder if you would see any additional performance boost from using new Function rather than eval?

I'm concerned about the security implications of directly exposing an eval (or Function) call that accepts arbitrary strings. You would need to introduce some kind of sanitizing on the server side that ensured all calls were pre-approved canvas API calls. I'm not confident that I could write such a thing properly, without leaving any exploits. Parsing the entire string to sanitize it would almost certainly mitigate the performance gains to some extent. I'm honestly not sure whether it would still be faster or not.

You might want to create your own fork, either from master or from mine. You can try replacing the current logic with your StringBuilder mechanism, and see what you can do in terms of security. I'd be more than happy to step aside and withdraw my PR in favor of yours if you build a better system.

[matteo-prosperi]

@WilStead I tried the new Function approach and the drawing time seems the same as when using eval.

I would love to be able to contribute an implementation but I don't have nearly enough understanding of JavaScript or TypeScript to help here.

I am not sure what the security implications of running code from a string would be here. It is my understanding that there is no "server side" involved running this code. I tried reading articles about eval and new Function and I understand that it should be somewhat safe to eval code that you generate yourself. But I am not an expert.

[jorolf]

Note that @matteo-prosperi's approach only works when you're using simple types like numbers or strings. However when you're using types like arrays or images it breaks/doesn't work because you can't convert them into executable javascript code. (Well maybe you could but that would be really inefficient)

[WilStead]

@matteo-prosperi It is generally safe to eval code you write yourself. The problem here is that in a Blazor JS interop scenario, you must expose the method that the C# code calls on a global variable. That means anybody viewing a website with the component could open their console and call that function with any string they like.

On a production website that involves non-trivial business logic, a skilled hacker could cause practically any amount of trouble with the power to execute any javascript they please.

[matteo-prosperi]

@jorolf Yes, I agree. I didn't mean to suggest a solution, just to provide a performance baseline for batch operations and let you know that it is reasonable to achieve at least a 5X performance boost for some scenarios by playing with the way javascript invokations are performed.

@WilStead I am not sure I understand your point. I don't think any page, probably even one without any javascript at all, is safe from a user injecting code in his own browser instance using the developer tools. But this is not my field of expertise, so I will not push this further. Also I am not really suggesting to go ahead with such an implementation, I just wanted to provide a performance reference other more elegant implementations can try to reach or surpass.

eval doesn't seem to actually be needed to achieve better performances. The code below also completes the drawing in roughly 2 seconds.

    callBatch: function (canvas, batchedCalls) {
        const ctx = canvas.getContext('2d');
        var arrayLength = batchedCalls.length;
        for (var i = 0; i < arrayLength; i++) {
            if (batchedCalls[i][1])
                ctx[batchedCalls[i][0]].apply(ctx, batchedCalls[i].slice(2));
            else
                ctx[batchedCalls[i][0]] = batchedCalls[i][2];
        }
    }

During the weekend I can try to pinpoint exactly what is slowing down this implementation.

[matteo-prosperi]

@WilStead I investigated this more to try identifying what are the reasons for the perfomance differences.
As you can see, in my example above, I was using a List<object[]> (coverted to object[][] before passing it to javascript) to represent the batch of operations.

When changing the data type from object[] to your CanvasBatchedCallInfo struct (no other change) the drawing time goes up to 7-10 seconds. The time remains high even if I remove the code on the javascript side *leaving just an empty javascript method), suggesting that converting classes and structs to javascript objects is very onerous (much more than converting arrays).

The number of calls on the C# side (FillRectAsync->CallMethodAsync->BatchMethodCallAsync->BatchCallAsync) also seems to have a significant effect on performances (I don't know how much this could be caused by Blazor not being optimized yet).

Finally adding/removing async and locking on the SemaphoreSlim doesn't seem to have any significant effect on performances.

I hope you will find this information useful. :-)

[jorolf]

Just out of curiosity: is there a performance difference between using a class or a struct for CanvasBatchedCallInfo?

[matteo-prosperi]

@jorolf I tried using both a class and a struct and it didn't really make any difference.

[WilStead]

@matteo-prosperi That is definitely helpful information. I took some time to refactor the code to use object arrays instead of the custom struct, and to reduce the method call chain by a couple of steps. Hopefully that will lead to a significant performance boost.

Unfortunately, I also attempted to update my PR to the latest blazor preview version (3.0.0-preview4), and I am no longer able to build and run the project successfully. The new blazor js throws websocket-related client disconnection errors.

It may be wise to wait until the final asp.net 3.0 release. We should be getting a release date at Build next month, supposedly sometime during the second half of this year. Even though client-side blazor isn't expected for the first release, server-side is supposed to be included. Hopefully the API shape at least will be less fluid for both scenarios. Then I can update this to a PR that targets a release version, instead of preview bits that keep changing.

[galvesribeiro]

Hello @WilStead

Can you update the PR to the preview 4?

Thank you!

[WilStead]

@galvesribeiro I did some work to update the project to preview 4, but it seems the CI build fails now. I wasn't able to see detailed logs to troubleshoot. Everything builds and the test projects are running correctly on two of my own systems.

[galvesribeiro]

Thank you so much for the update @WilStead!

Please just remove the reference to MyGet and we can move forward and merge it.

Thanks!

[galvesribeiro]

Thank you very much @WilStead for this huge PR! 💃

Will make a release later tonight!

Thanks!