Skip to content

Attributes Respected By Serializer #43

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Mar 7, 2025

Conversation

magiccodingman
Copy link
Owner

@magiccodingman magiccodingman commented Mar 6, 2025

Goal is to remove system reflections from the process of attributes being respected. Utilizing only the serializer. This will centralize logic, simplify the codebase, and provide considerable performance improvements.

Massive Serialization and Performance Refactor

This PR introduces significant optimizations, structural simplifications, and performance enhancements by overhauling the serialization logic, caching mechanisms, and reflection processes within the project.

Argument Passing Fix

The core issue addressed involved custom serialization logic for JavaScript interop calls (CallJsAsync). Previously, dynamic objects were passed without explicit typing, causing reflection-heavy serialization overhead and unpredictable handling by Blazor’s default serializer.

Problem

Initially, arguments were passed as loosely typed object arrays (object[]). However, our custom serialization logic required type information at runtime to handle attribute-based serialization rules. Using object[] directly bypassed this and incurred unnecessary reflection costs.

Solution

Introduced a strongly-typed approach using a generic TypedArgument<T> and the ITypedArgument interface:

public interface ITypedArgument
{
    JsonElement SerializeToJsonElement(MagicJsonSerializationSettings? settings = null);
}

internal async Task CallJsAsync(string functionName, CancellationToken token, params ITypedArgument[] args)
{
    object[] serializedArgs = MagicSerializationHelper.SerializeObjects(args, new MagicJsonSerializationSettings { UseCamelCase = true });
    await _jsModule.InvokeVoidAsync(functionName, token, serializedArgs);
}

internal async Task<T> CallJsAsync<T>(string functionName, CancellationToken token, params ITypedArgument[] args)
{
    object[] serializedArgs = MagicSerializationHelper.SerializeObjects(args, new MagicJsonSerializationSettings { UseCamelCase = true });
    return await _jsModule.InvokeAsync<T>(functionName, token, serializedArgs);
}

Solution

  • Each argument type implements ITypedArgument for efficient, type-specific serialization.
  • MagicSerializationHelper efficiently converts objects into serialized JSON elements, properly handling arrays and primitives, avoiding unnecessary deserialization/re-serialization cycles.

Schema Name Method and Attribute Reflection Optimization

To dramatically reduce reflection overhead, the schema name lookup (SchemaHelper.GetSchemaName<T>()) and column mapping logic have been optimized through caching strategies. Reflection is now used only once per type, significantly boosting runtime performance.

Schema Name Caching

  • Schema names and property attributes are cached after the first reflection lookup.
  • Eliminated repeated runtime reflections in subsequent calls.

MagicAttribute Struct Improvements

  • Introduced MagicPropertyEntry and related structs, significantly minimizing memory overhead.
  • Attribute lookups (like MagicPrimaryKeyAttribute, MagicIndexAttribute, etc.) utilize cached properties for instant retrieval.

Removal of Reflection-Heavy Column Name Lookup

Previously, MagicColumnNameDesignatorAttribute used reflection to dynamically detect column names. This complexity was unnecessary and inefficient.

  • Introduced IColumnNamed interface:
public interface IColumnNamed
{
    string ColumnName { get; }
}
  • Attributes directly implement this interface, removing runtime reflection overhead.

MagicContractResolver Refactor

Significantly streamlined serialization logic, enabling clear handling of arrays, primitive types, and complex objects:

  • Enhanced handling of simple types, collections, and complex types explicitly.
  • Ensured strings and primitives are serialized directly, preventing infinite recursion.
  • Ensured camelCase policy is respected consistently when enabled.

Attribute-Based Optimizations & Caching

MagicPrimaryKeyAttribute

  • Updated methods (UpdateAsync, UpdateRangeAsync, GetByIdAsync) to use a cached primary-key lookup, reducing reflection overhead.
  • Enhanced UpdateRangeAsync and DeleteRangeAsync to use LINQ streaming for improved performance and memory efficiency.

Deprecated Features & Cleanups

  • Marked encryption attributes and methods as obsolete with explicit warnings due to their insecure implementation. Future PR will entirely remove deprecated encryption logic.
  • Removed redundant and inefficient helper classes, notably ManagerHelper.

Query and Expression Logic Updates

  • Updated expression-building logic (MagicQuery) to utilize the optimized PropertyMappingCache.
  • Simplified query-related attribute checking, further enhancing performance and reducing complexity.

Key Performance and Maintainability Benefits

  • Drastically reduced runtime reflection, minimizing overhead and memory footprint.
  • Simplified attribute handling through a centralized and cache-friendly approach.
  • Ensured clear, explicit serialization behavior.
  • Improved maintainability by centralizing serialization logic and attribute management.

Important Notes for Future Work

  • The Encryption feature has been explicitly marked obsolete and will be entirely removed in future updates.
  • Future PRs should revisit the expression builder logic to leverage these new caching and serialization improvements for even greater performance gains.

@magiccodingman magiccodingman requested a review from yueyinqiu March 6, 2025 17:32
@magiccodingman magiccodingman self-assigned this Mar 6, 2025
@magiccodingman
Copy link
Owner Author

@yueyinqiu No actual changes, was just setting this PR up and moving us to a new discussion thread since the last PR achieved its intended goal.

@magiccodingman
Copy link
Owner Author

@yueyinqiu This refactor was significantly larger than anticipated. I'm nearly done, but need to get off for the night. I've pushed the changes as there's a lot. This is likely even larger than the last PR I submitted. Performance improvements will be extremely noticeable. As this wasn't as simple as originally suspected with just change on AddAsync. it required an entirely new mechanism, structures, methods, caching, and removal of lots of system reflections to do this.

This'll be a very large refactor when I'm done that we'll need to test but the changes are good. very good.

@magiccodingman
Copy link
Owner Author

@yueyinqiu Need some assistance with testing. My Visual Studios is messing up and it's pretty late for me so I'll fix it tomorrow, but the NET 9 project is blowing up my PC. I think I just need to update VS to support NET 9, but I'll do that in the morning. This should be working.

This refactor was significantly larger than expected. Took me a lot longer than expected. Altering AddAsync was NOT as simple as I had hoped haha. To change just that aspect, I had to alter the entire core functionality of how arguements are passed, type arguments, CallJsAsync, tons of new serializer capabilities, tons of new caching capabilities, total stripping of json serialization rights from Blazor and taking full control, and a lot more. This was no easy task, but it's done!

This'll open a world of possibilities up for this project in the future. We should see massive performance increases too hopefully because in theory, everything I've done should be pretty massive. Plus all caching now occurs on startup. It still does caching if it misses something (somehow if that happened) but that's just fallback strategies. Reflections are basically utilized now (or should be) only at startup. And after that, everything is pure cache lookups. Plus all this should be AOT friendly as well.

I put all the documentation of changes in the PR comment up top. I think I got everything documented? But it was so many changes that were so fundamental to the entire platform that I'm unsure exactly if I documented everything. I mean even how attributes are targeted, column names are swapped, literally everything was changed.

Should be good to go if our testing works.

@yueyinqiu
Copy link
Collaborator

I would just push into this branch since we won't have conflicts.

@magiccodingman magiccodingman merged commit 541420c into master Mar 7, 2025
@magiccodingman
Copy link
Owner Author

Let me know when you have time to test this. I'll do some testing in the morning when I'm back online. If we both have no issues, may be time to make a release for this update. When stable, it's significant enough to get the updates out we've both been working on.

@yueyinqiu
Copy link
Collaborator

yueyinqiu commented Mar 7, 2025

All tests in OpenTest succeed on my machine now:

image

And the AddTest still fails due to:

  • [MagicNotMapped] Ignored is still included
  • [MagicIndex("Renamed")] ShouldBeRenamed is still stored as shouldBeRenamed

@magiccodingman
Copy link
Owner Author

Oh good test. I really need to get your unit tests running on my side. My VS studios are still in an update, but i know exactly what's going on now.

Basically I had to make a really weird system to not break users current code because what I didn't know is that System.Text.Json automatically uses camel case and that's what occurs when you pass into the CallJs within blazors default behavior. So I had to enforce camel case.

But I need to enforce camel case to keep those defaults, but respect any magic attribute is never brought along for camel case and instead must never follow any default serialization settings. Plus the ignore attribute follows new rules now, but it seems like my VS update is letting me use NET 9. Let me see if I can fix this before I turn in.

@yueyinqiu
Copy link
Collaborator

yueyinqiu commented Mar 7, 2025

Oh, maybe I didn't make that clear. System.Text.Json doesn't use camelcase by default. It's blazor jsinterop that pass such option.

And actually our former version does not use camelcase, since we have converted the object to a dictionary, so it won't follow that policy.

@magiccodingman
Copy link
Owner Author

I think I know the issue btw. The caching system I made is pretty busted after using your unit tests. Will need to fix the rest later.

@magiccodingman
Copy link
Owner Author

magiccodingman commented Mar 8, 2025

Unit Tests

Got your unit tests running on my end—didn't even know you could do unit testing like this in Blazor! This is seriously amazing, and I should've looked into it way sooner. Huge help!

That said, we’ll need to refactor things to align with the new deserialization system. Anything running through deserialization now needs to follow this logic:

internal async Task<T> CallJsAsync<T>(string functionName, CancellationToken token, params ITypedArgument[] args)
{
    var settings = new MagicJsonSerializationSettings() { UseCamelCase = true };
    object[] serializedArgs = MagicSerializationHelper.SerializeObjects(args, settings);
    var resultJsonElement = await _jsModule.InvokeAsync<JsonElement>(functionName, token, serializedArgs);
    string resultJson = resultJsonElement.GetRawText();
    return MagicSerializationHelper.DeserializeObject<T>(resultJson, settings);
}

Right now, a lot of the tests are failing, even though similar ones are passing in my own (admittedly less robust) unit tests. Anything involving Magic IndexDB needs specialized logic to match real runtime behavior.

I didn’t even get far adjusting your tests because it's failing here:
protected async ValueTask<string> RunTestPageMethodAsync(...)

I think we need new unit tests built around the updated deserialization strategy. Fixing the current single-record tests won't be enough—they don’t fully exercise the logic that matters. The new system works much closer to Newtonsoft and System.Text.Json, but because we’ve taken over deserialization entirely, other methods won’t work unless they go through Magic IndexDB. I’ve got some code for unit testing startup services I can dig up to help with that.

Solutions & Changes

Our deserializer is weirdly specific to Blazor now. We return a JsonElement from InvokeAsync because Blazor requires deserialization at some level, but we’ve overridden its ability to do it. Now everything is handled dynamically by our own system, eliminating runtime dictionary-based conversions.

The extra step—returning a JsonElement, then passing it through our deserializer—ensures we maintain flexibility and avoid type mismatches. The deserializer can handle non-JsonElement types, but this approach turned out to be way more involved than I expected. If I had realized how deep this would go, I probably would've just built cached types at startup instead of replacing everything with a new strategy.

But I was already too deep, man. Kept telling myself, "Just one more hour and I’ll figure it out." Next thing I knew, I was banging my head against the wall for two straight days 🙃. Probably should've tapped out sooner, but hey—now we have way more control over the system!

Final Update

This should be working but without all the unit tests and rebuilding them, I didn't have time to test every edge case like your unit tests did.

I actually got a bit too wrapped up in this and need to focus on other development for the next week. I can dump a bit of time here or there over the next week or 2, but I'll be hunkering down on my other projects. Keep me updated on the unit tests being fixed and hopefully this works out much better for us moving forward. May just have a bit of kinks to smooth out but it will do us very well.

There's also a great deal of caching techniques that I've learned. Oh my god I never want to learn about caching delegate strategies again hahaha. I just really didn't want to leave master in shambles after that last PR update since I didn't realize that the code was pretty bunked on aspects I didn't test.

But one thing I can say. I've likely built one of the most optimized custom blazor WASM based serialization systems that exists or at least has been open sourced lol. As far as I can tell, there now should be effectively zero performance impact on any realistic scenario we could throw at it.

This is as fast as we can go with Magic IndexDB.

I don't hate that we now have this, but I hate the fact I ended up building it bwahaha.

  • No rush but if you have time to help support repairing your unit testing library with the new changes, it would be appreciated! I'm sorry that my changes broke it, I didn't think it would and I was confused for a long time until I realized the unit tests were broke, not the code.

  • Changes are in main

@magiccodingman
Copy link
Owner Author

Oh, maybe I didn't make that clear. System.Text.Json doesn't use camelcase by default. It's blazor jsinterop that pass such option.

And actually our former version does not use camelcase, since we have converted the object to a dictionary, so it won't follow that policy.

Also I forgot to mention, this aspect you brought up was something I learned while building this haha. I had no clue that the blazor default interopt uses camel case. I had to make it replicate similarly for the serializer, but respect no camel case is ever respected on any magic property appended by our attributes. Those will follow no policy. That was a weird one to figure out and fix because you respect some things but not others. And it was mind bending to figure out on the reader. Because the reader works almost backwards to how you'd think it'd work.

@yueyinqiu
Copy link
Collaborator

yueyinqiu commented Mar 8, 2025

Ok I would check this.

And there is another question worth confirming for CallJsAsync:

internal async Task<T> CallJsAsync<T>(string functionName, CancellationToken token, params ITypedArgument[] args)
{
    var settings = new MagicJsonSerializationSettings() { UseCamelCase = true };
    object[] serializedArgs = MagicSerializationHelper.SerializeObjects(args, settings);
    var resultJsonElement = await _jsModule.InvokeAsync<JsonElement>(functionName, token, serializedArgs);
    string resultJson = resultJsonElement.GetRawText();
    return MagicSerializationHelper.DeserializeObject<T>(resultJson, settings);
}

InvokeAsync<JsonElement> will automatically do deserialization, and GetRawText() is actually a process of serialization. So it seems that it would be better to do something like (I haven't check the code and just assume that serializedArgs is a string[]):

internal async Task<T> CallJsAsync<T>(string functionName, CancellationToken token, params ITypedArgument[] args)
{
    var settings = new MagicJsonSerializationSettings() { UseCamelCase = true };
    string[] serializedArgs = MagicSerializationHelper.SerializeObjects(args, settings);
    string resultJson = await CALL_JS_WITH_STREAM_OR_BYTE_ARRAY(functionName, token, serializedArgs);
    return MagicSerializationHelper.DeserializeObject<T>(resultJson, settings);
}

@magiccodingman
Copy link
Owner Author

magiccodingman commented Mar 8, 2025

I'm familiar that Blazors JsInvoke does deserialization. That was 1/2 the battle for me actually and was a pain in the butt. But I wasn't too worried because I 100% agree. I'll be moving in the future towards a js call with a stream or byte array that doesn't go through deserialization by the default InvokeAsync.

It just seemed like more of a refactor than I was willing to do at the time to look into that since the serializer was already a massive refactor. And it may requrie some JS return changes potentially, so I left it just deserializing to JsonElement to bypass as much as I could afford (time wise) right now.

Because it's still an extremely small deserialization and this improvement is so large, that we're still leagues above where we were just a week ago.

@magiccodingman
Copy link
Owner Author

magiccodingman commented Mar 8, 2025

Also I think the JS Stream version does partial deserialization too right? I can't remember. Worth looking into, but I was also going to run into that when I got there. Because I actually am trying to figure out if it's faster to respond via InvokeAsync or if it's actually better to utilize dotnet invoke from the JS side where there were readied opened paths waiting for piped returns. Will be fun to look into.

@yueyinqiu
Copy link
Collaborator

yueyinqiu commented Mar 8, 2025

Hmm... I don't got a failure on RunTestPageMethodAsync, but got something like:

image

It's because an exception occurred when running SingleRecordBasicTestPage.Add, which seems to happen when serializing the nested object.

(I'm using the master branch.)

@magiccodingman
Copy link
Owner Author

Yea, that's what I was talking about earlier. I wasn't having that issue in my testing, but that's because my unit tests use the new serialization system. I didn't have enough time to fully figure out why that was happing on the unit test though. I couldn't get it to repeat when using blazor directly. I think the tests need to be refactored to use the new Magic Serializer. And we may need some test classes to replicate real inserts and reads of an initiated environment.

@magiccodingman
Copy link
Owner Author

The new serializer refactor was so massive. That a lot of things have to line up for it to work properly. I when I tried investigating it. It was something about the serializers caching on the cached getter in the new struct for MagicPropertyEntry, but I couldn't figure out what in the unit test was calling it in an un authorized way.

@magiccodingman
Copy link
Owner Author

You know now that I think about it. The caching on the first time certain actions are called. large assembly calls are made to pre-cache tons of work on startup on user behalf. Also it was kind of necessary for me to get certain things to work right in Blazor. And I wonder if the service startup of the unit test isn't working with my newly created methods?

@yueyinqiu
Copy link
Collaborator

yueyinqiu commented Mar 8, 2025

Ah... I found the reason. It's because a property of the nested object's is readonly... I've modified this in #52.

By the way, the unit test actually starts a blazor web application (E2eTestWebApp), and use a real web browser to access it. So there should be no differences from the actual scene.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants