trampster / JsonSrcGen

Json library that uses .NET 5 Source Generators
MIT License
148 stars 4 forks source link

Decimal type added and Project Benchmarks added #44

Closed hugobritobh closed 3 years ago

hugobritobh commented 3 years ago

Unit Tests Outcomes 347 Passed 4 Failed (It seems that an error occurred just for me, so it could be a time zone) Do some testing with internationalization (Example: DecimalPropertyTests)

CLAassistant commented 3 years ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: hugobritobh
:x: Hugo de Brito Valadares Rodrigues Alves


Hugo de Brito Valadares Rodrigues Alves seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

trampster commented 3 years ago

Nice Work :)

Just few minor things:

trampster commented 3 years ago

I tried benchmarking the IsNumber method vs the Switch statement and the switch statement is a little faster. (inlining didn't make any difference.)

Method Mean Error StdDev
IsNumberMethod 21.15 ns 0.034 ns 0.028 ns
IsNumberInlineMethod 21.36 ns 0.079 ns 0.074 ns
SwitchStatement 15.78 ns 0.050 ns 0.045 ns

The benchmark code is here: https://gist.github.com/trampster/117692f09168f262a995b7bb95db29a9

switch statements like this can be compiled into a jump table, which can be executed in a jump instruction. This can beat a two comparisons method like is required in IsNumber.

I think we should leave the switch statements in.

hugobritobh commented 3 years ago

I tried benchmarking the IsNumber method vs the Switch statement and the switch statement is a little faster. (inlining didn't make any difference.)

Method Mean Error StdDev IsNumberMethod 21.15 ns 0.034 ns 0.028 ns IsNumberInlineMethod 21.36 ns 0.079 ns 0.074 ns SwitchStatement 15.78 ns 0.050 ns 0.045 ns The benchmark code is here: https://gist.github.com/trampster/117692f09168f262a995b7bb95db29a9

switch statements like this can be compiled into a jump table, which can be executed in a jump instruction. This can beat a two comparisons method like is required in IsNumber.

I think we should leave the switch statements in.

It could be something from my processor. So, I even made a project for people to test: BenchmarksNumber