Skip to content
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

Use interface attributes #241

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

devtekve
Copy link

This PR brings support to defining DynamoDb attributes on the interfaces instead of only classes. This doesn't happen by default on C# as attributes on interfaces are not projected to the classes that implement them. However, through some reflection we can get that information.

Example

public interface IDynamoEntity
{
    [DynamoDbProperty("UserId")]
    string UserId { get; set; }
}

internal class UserEntity : IDynamoEntity
{
    // This is already known as a property because the interface defined it as attribute.
    public string UserId { get; set; }

    [DynamoDbProperty("Email")
    public string Email { get; set; }
}

internal class PermissionsEntity : IDynamoEntity
{
    // This is already known as a property because the interface defined it as attribute.
    public string UserId { get; set; } 

    [DynamoDbProperty("IsAdmin")]
    public bool IsAdmin { get; set; }
}

Performance

However, I need somebody that knows how to use properly benchmarkdotnet to validate the results. I added a test case and I have no explanation for the numbers. I ran this many times and this implementation is somehow more performant... I honestly don't know how or why.

BenchmarkDotNet v0.13.12, macOS Sonoma 14.3.1 (23D60) [Darwin 23.3.0]
Apple M1 Pro, 1 CPU, 10 logical and 10 physical cores
.NET SDK 8.0.201
  [Host]     : .NET 8.0.2 (8.0.224.6711), Arm64 RyuJIT AdvSIMD
  DefaultJob : .NET 8.0.2 (8.0.224.6711), Arm64 RyuJIT AdvSIMD


| Method                          | EntitiesCount | Mean         | Error      | StdDev       | Gen0      | Gen1      | Gen2      | Allocated   |
|-------------------------------- |-------------- |-------------:|-----------:|-------------:|----------:|----------:|----------:|------------:|
| EfficientDynamoDb               | 10            |     26.44 us |   0.431 us |     0.382 us |    2.7466 |    0.0610 |         - |    17.17 KB |
| EfficientDynamoDb-FromInterface | 10            |     18.75 us |   1.560 us |     4.525 us |    1.2207 |         - |         - |     7.56 KB |
| aws-sdk-net                     | 10            |    205.65 us |   4.073 us |     8.768 us |   51.2695 |   13.1836 |         - |   314.92 KB |
| EfficientDynamoDb               | 100           |    215.11 us |   3.996 us |     3.925 us |   19.5313 |    0.4883 |         - |   119.86 KB |
| EfficientDynamoDb-FromInterface | 100           |     30.28 us |   0.605 us |     0.595 us |    3.8452 |    0.1831 |         - |    23.73 KB |
| aws-sdk-net                     | 100           |  2,446.21 us |  47.644 us |    63.603 us |  476.5625 |  234.3750 |         - |  2956.09 KB |
| EfficientDynamoDb               | 1000          |  2,067.24 us |  27.943 us |    24.770 us |  183.5938 |   89.8438 |         - |  1146.55 KB |
| EfficientDynamoDb-FromInterface | 1000          |    236.34 us |   2.681 us |     2.507 us |   30.2734 |    8.7891 |         - |   185.47 KB |
| aws-sdk-net                     | 1000          | 41,990.48 us | 816.181 us | 1,061.266 us | 4833.3333 | 1250.0000 | 1083.3333 | 29341.08 KB |

This commit enhances the way DdbClassInfo handles types. It adds support for type interfaces by using a Stack to store them. This change ensures that all interfaces of a given type are correctly processed and taken into account.
A new entity MixedEntityFromInterface has been added along with its creation method in EntitiesFactory. This entity is being used in QueryEntityComparisonBenchmark for performance testing. The entity includes several properties like list, map, hashset and basic types.
@firenero
Copy link
Contributor

While I can see how this can be beneficial for someone, I'm not a fan of this feature in its current state. Currently, there is a simple rule - propertied with DynamoDbProperty attribute are saved to DynamoDB while all others are not.

This PR breaks this "rule" and makes things more complicated. Since there is no DynamoDbIgnore attribute (and no intention to add it as of now), there is no way to exclude properties from being serialized if you implement an interface with properties marked with DynamoDbProperty.

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