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

Test Improvements & Benchmarks for SqlKata Compiler #738

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

faddiv
Copy link

@faddiv faddiv commented Feb 8, 2025

Hi Ahmad,

We've been using SqlKata in our project, and I’ve started exploring its internals.
I thought the compiler part could be improved significantly both for speed and memory usage.
However, to rewrite it safely, I first wanted to improve the tests and add some benchmarks.

This PR shows what I did in this regard. Please review this PR, and let me know if you are interested in any part of my work. (I also have a branch in which I implemented a new compiler, just enough to run the benchmarks.)

How to run the benchmark

In IDE:

  • Set QueryBuilder.Benchmarks as the startup project.
  • Set Build mode to release. (BanchmarkDotNet only runs on release compilation)
  • Start the project without debugging.
    Command line:
    From the solution folder (on Windows):
    dotnet run -c Release --project .\QueryBuilder.Benchmarks\QueryBuilder.Benchmarks.csproj

The reports are generated into the .\BenchmarkDotNet.Artifacts\ folder at the solution level.

Meanwhile, it is recommended to close everything else, and do as little as possible so other processes don't affect the results.

Summary of Changes:

  • I broke down the tests that check the same input with multiple engines, with Theory and InlineData attributes. This way it can be seen if anything fails separately, and better visible if any test is missing.
    • At some places, I didn't do this breakdown everywhere since I was unsure if it was really worth it. (Base class functionality was tested.)
    • I replaced TestCompilersContainer completely. I was not sure in which direction you wanted to move these tests, but I saw somewhere that you wished to deprecate them.
  • Created separate tests for the bindings.
  • Created some benchmarks with three example queries.
    • These benchmarks only test the compiler part.
    • In benchmarks, I usually create pre-tests to validate whether they return the correct results, which I did here as well.
  • Also, it was my goal, to make the compiler easily replaceable in the tests.

Looking forward to your feedback!

@ahmad-moussawi
Copy link
Contributor

Hey @faddiv thanks for your contribution, I would take a bit of time to review it, but it seems interesting,
can you please add to the PR description how can I run the benchmarks?

@faddiv
Copy link
Author

faddiv commented Feb 13, 2025

Hi @ahmad-moussawi ,

I added the description. It simply can run with the following command: dotnet run -c Release --project .\QueryBuilder.Benchmarks\QueryBuilder.Benchmarks.csproj

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