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

🎉 create project #4

Merged
merged 30 commits into from
Sep 22, 2023
Merged

🎉 create project #4

merged 30 commits into from
Sep 22, 2023

Conversation

Odonno
Copy link
Contributor

@Odonno Odonno commented Aug 2, 2023

This is a naive implementation for a client SDK in .NET written in C#. (.NET standard 2.1)

Some details about the changes in this PR:

  • it is entirely written in C# as it is the main language in the .NET landscape
  • it is written following testing best practices:
    • TDD, leveraging:
      • clean code/architecture
      • regression testing
      • adding new features and tests easily
    • xUnit and FluentAssertions libraries
    • a vast majority of tests are integration tests, ensuring compatibility with a concrete SurrealDB version
    • each integration test is using the same SurrealDB instance (localhost on port 8000)
  • there may be edge cases not covered
  • trying to have the same API as the Rust driver (Connect, Select, Create, Update, Delete, Signin, etc…)
  • plus the specific usage for dotnet developers : extension method AddSurreal() for DI
    • use URL parameter entry point like the SurrealDb Rust constructor
    • “Options” model declaration
    • Multiple instances vs. Default instance
    • ConnectionString
  • a Benchmark project to display CPU and RAM usage and improve performance over time
    • current benches on basic functions for multiple client implementations
      • HttpClient (default)
      • HttpClient (with HttpClientFactory)
      • Websocket (could be interesting to split between TEXT and BINARY protocols)
  • there are multiple GitHub workflows
    • main workflow = build & test
    • release workflow = build in RELEASE mode and publish NuGet package
    • run benchmark in CI to detect performance regression, mainly for PR
  • the SurrealDb NuGet is ready to publish as it is, following the naming convention for package name and namespaces
  • Examples projects
    • A classic ASP.NET Core Web API from the default template (WeatherForecast API)
    • A basic Console app
    • TODO : more examples (Blazor, TodoMVC, etc…)
  • TODO : more client implementation could be possible : embedded (memory, rocksdb), WASM binding, etc…
  • Some notes about the SurrealDbResponse type:
    • Would it be better to avoid memory allocation at all? (i.e. using the JsonDocument directly without parsing)
    • Same question but with caching of the GetResult function on lazy evaluation?

@Odonno
Copy link
Contributor Author

Odonno commented Aug 2, 2023

Here is the expected output of the benchmarks:


BenchmarkDotNet v0.13.6, Windows 11 (10.0.22621.2070/22H2/2022Update/SunValley2)
AMD Ryzen 9 5900X, 1 CPU, 24 logical and 12 physical cores
.NET SDK 7.0.306
  [Host]     : .NET 6.0.20 (6.0.2023.32017), X64 RyuJIT AVX2
  DefaultJob : .NET 6.0.20 (6.0.2023.32017), X64 RyuJIT AVX2


Select

Method Mean Error StdDev Gen0 Gen1 Allocated
HttpClient 2,035.276 ms 7.2211 ms 6.7547 ms - - 290.2 KB
HttpClientWithFactory 1.181 ms 0.0058 ms 0.0054 ms 17.5781 5.8594 285.82 KB

Create

Method Mean Error StdDev Gen0 Allocated
HttpClient 2,030,668.2 μs 2,753.79 μs 2,149.98 μs - 154.23 KB
HttpClientWithFactory 476.5 μs 3.15 μs 2.95 μs 2.9297 54.87 KB

Upsert

Method Mean Error StdDev Gen0 Allocated
HttpClient 2,033,185.9 μs 7,766.54 μs 6,884.83 μs - 67.75 KB
HttpClientWithFactory 459.0 μs 1.19 μs 1.05 μs 3.4180 58.4 KB

Delete

Method Mean Error StdDev Gen0 Allocated
HttpClient 2,037,661.2 μs 8,955.45 μs 8,376.94 μs - 30.77 KB
HttpClientWithFactory 309.5 μs 0.99 μs 0.93 μs 0.4883 9.3 KB

Query

Method Mean Error StdDev Gen0 Allocated
HttpClient 2,036,436.5 μs 5,340.32 μs 4,734.06 μs - 32.09 KB
HttpClientWithFactory 302.0 μs 1.11 μs 0.98 μs 0.4883 8.89 KB

@Odonno
Copy link
Contributor Author

Odonno commented Aug 6, 2023

I updated the benchmark based on the new Websocket implementation and some changes on the Http implementation.

N.B. 1: there are still many ways to improve the performance of this library, at least in terms of memory allocations. Will do some performance improvements it in the next coming weeks.

N.B. 2 : The default Http client looks like it is the best solution but it is due to many factors (caching, single operation per benchmark invocation, single instanciation, etc...). I may need to create another benchmark with a complete workflow (authentication + CRUD + parallelism) to have a proper benchmark closer to a real application use.


BenchmarkDotNet v0.13.6, Windows 11 (10.0.22621.2070/22H2/2022Update/SunValley2)
AMD Ryzen 9 5900X, 1 CPU, 24 logical and 12 physical cores
.NET SDK 7.0.306
  [Host]     : .NET 6.0.20 (6.0.2023.32017), X64 RyuJIT AVX2
  DefaultJob : .NET 6.0.20 (6.0.2023.32017), X64 RyuJIT AVX2


Select

Method Mean Error StdDev Gen0 Gen1 Allocated
Http 335.1 μs 1.34 μs 1.05 μs 0.4883 - 9.83 KB
HttpWithClientFactory 1,036.2 μs 8.33 μs 7.79 μs 15.6250 3.9063 253.87 KB
WsText 4,626.6 μs 49.26 μs 43.67 μs 39.0625 15.6250 2675.36 KB
WsBinary NA NA NA NA NA NA

Create

Method Mean Error StdDev Gen0 Allocated
Http 476.6 μs 8.17 μs 7.64 μs 2.9297 53.78 KB
HttpWithClientFactory 464.9 μs 3.95 μs 3.70 μs 2.9297 55.71 KB
WsText 807.3 μs 8.71 μs 8.15 μs 2.9297 58.21 KB
WsBinary NA NA NA NA NA

Upsert

Method Mean Error StdDev Gen0 Allocated
Http 462.0 μs 2.33 μs 2.18 μs 3.4180 56.85 KB
HttpWithClientFactory 468.9 μs 2.37 μs 1.98 μs 3.4180 58.53 KB
WsText 866.1 μs 4.48 μs 3.97 μs 2.9297 60.85 KB
WsBinary NA NA NA NA NA

Delete

Method Mean Error StdDev Gen0 Allocated
Http 303.6 μs 4.53 μs 4.02 μs - 7.61 KB
HttpWithClientFactory 312.2 μs 1.61 μs 1.42 μs 0.4883 9.3 KB
WsText 314.4 μs 1.74 μs 1.46 μs 0.4883 8.26 KB
WsBinary NA NA NA NA NA

Query

Method Mean Error StdDev Gen0 Gen1 Allocated
Http 475.0 μs 6.82 μs 6.38 μs 0.9766 - 16.11 KB
HttpWithClientFactory 1,624.4 μs 8.44 μs 7.48 μs 13.6719 3.9063 451.87 KB
WsText 3,130.2 μs 24.06 μs 18.78 μs 27.3438 7.8125 1521.82 KB
WsBinary NA NA NA NA NA NA

@DanielBaumert
Copy link

DanielBaumert commented Aug 7, 2023

@Odonno
Copy link
Contributor Author

Odonno commented Aug 7, 2023

Hi @DanielBaumert

Yes it is. My mistake. I fixed it.
I also surely need to use UriBuilder to avoid this dirty concat myself.

Thank you again for seeing this.

@DanielBaumert
Copy link

I hope that I am correct.
The license Apache 2 and MIT are both incompatible or ... ?
(Superpower - Apache-2.0 license)
(Websocket.Client - MIT license)

@Odonno
Copy link
Contributor Author

Odonno commented Aug 12, 2023

Hi Daniel.

Those 2 licenses are different but they can be compatible.

Because I am writing a C# library, and because I am using those libraries underneath, there is no problem whatsoever as long as I respect the terms and requirements of each project license.

@Odonno Odonno marked this pull request as ready for review August 17, 2023 17:41
@DanielBaumert
Copy link

Hii :D

What are your thoughts on substituting each exception with a discriminated union?

I'm inquiring about this because Exception handling in .NET is not straightforward for a few reasons:
a) The compiler doesn't provide any information about unhandled exceptions.
b) If you forget to document an exception, the end user won't have any information about whether you're throwing an exception or not.
c) Due to reason 'a,' the runtime of my application is at risk because not all exceptions need to be handled.

@Odonno
Copy link
Contributor Author

Odonno commented Sep 19, 2023

Hi @DanielBaumert

This is an excellent question and I can understand your point of view.

Exception handling is a very familiar pattern in C#. So most C# developers will expect to deal with exceptions rather than a discriminated union. On the contrary, if I was using F#, I would prefer discriminated union over exception.

Regarding b). It is indeed something that has to be documented. I will double check to see if I did not miss any comment.

Regarding c). Yes, this is true. But if the projects you use are well documented, I suppose that you could have an Analyzer at this time in your IDE to prevent uncaught exceptions. I say that but I don't have any myself at the momen if there was any... Hope this will be added in the future.

@Odonno
Copy link
Contributor Author

Odonno commented Sep 21, 2023

I completed the migration to 1.0.0. The only missing feature is Live Query which will come very soon.

I also added 2 benchmarks:

  • Cold Start to see how much time it takes to create a client + executing the Connect method (to connect to the SurrealDB server)
  • Scenario, a complete scenario to see how multiple executions on the same client & how parallelization can impact performance

As you can see, we can see a huge performance improvements (in time) in the WS implementation from beta-9 to 1.0.0. Note that I still did not made any performance improvement since the last benchmark.

You will certainly ask me why the 2s of cold start on WS implementation? This is indeed intriguing and I need to investigate to know why. Related to this issue Marfusios/websocket-client#100

Cold Start

Method Mean Error StdDev Allocated
HttpConstructor 14.80 ms 0.219 ms 0.183 ms 11.61 KB
HttpStaticConstructor 14.76 ms 0.222 ms 0.197 ms 11.87 KB
WsConstructor 2,052.21 ms 13.402 ms 12.536 ms 77.56 KB
WsStaticConstructor 2,060.05 ms 9.286 ms 8.686 ms 79.84 KB

Select

Method Mean Error StdDev Median Allocated
Http 14,756.6 μs 287.39 μs 282.26 μs 14,609.1 μs 10.33 KB
HttpWithClientFactory 14,674.6 μs 72.17 μs 56.34 μs 14,652.9 μs 13.49 KB
WsText 690.6 μs 13.73 μs 35.69 μs 678.3 μs 10.79 KB
WsBinary NA NA NA NA NA

Create

Method Mean Error StdDev Gen0 Allocated
Http 15,101.7 μs 297.39 μs 278.18 μs - 52.57 KB
HttpWithClientFactory 15,016.6 μs 163.85 μs 136.82 μs - 55.36 KB
WsText 816.6 μs 11.10 μs 9.84 μs 2.9297 55.46 KB
WsBinary NA NA NA NA NA

Upsert

Method Mean Error StdDev Gen0 Allocated
Http 14,968.5 μs 178.19 μs 148.80 μs - 26.8 KB
HttpWithClientFactory 14,892.0 μs 105.69 μs 82.52 μs - 28.94 KB
WsText 766.9 μs 15.09 μs 30.49 μs 0.9766 27.19 KB
WsBinary NA NA NA NA NA

Delete

Method Mean Error StdDev Allocated
Http 14,690.1 μs 120.34 μs 93.95 μs 8.39 KB
HttpWithClientFactory 15,376.9 μs 305.24 μs 725.43 μs 10.13 KB
WsText 632.2 μs 12.60 μs 12.38 μs 7.71 KB
WsBinary NA NA NA NA

Query

Method Mean Error StdDev Allocated
Http 15.595 ms 0.3010 ms 0.2816 ms 17.55 KB
HttpWithClientFactory 15.662 ms 0.3102 ms 0.3319 ms 17.93 KB
WsText 1.325 ms 0.0241 ms 0.0213 ms 19.16 KB
WsBinary NA NA NA NA

Scenario

Method Mean Error StdDev Allocated
Http 189.70 ms 2.732 ms 2.281 ms 173.12 KB
HttpWithClientFactory 190.94 ms 2.768 ms 2.453 ms 199.63 KB
WsText 11.17 ms 0.121 ms 0.108 ms 167.08 KB
WsBinary NA NA NA NA

@kearfy kearfy merged commit bc82bd7 into surrealdb:main Sep 22, 2023
@Odonno Odonno deleted the feat/project branch September 24, 2023 10:08
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.

None yet

3 participants