vechain / thor

A general purpose blockchain highly compatible with Ethereum's ecosystem
GNU Lesser General Public License v3.0
796 stars 247 forks source link

Increase Sqlite version #775

Closed MakisChristou closed 2 months ago

MakisChristou commented 3 months ago

This PR

Upgrade Golang + Golangci-lint

Upgrades all golang usage to 1.22 and the linter to 1.59.0. It's possible to upgrade golang usage to 1.22 and the linter to 1.59.1 by doing the following these commands:

go install golang.org/dl/go1.22.4@latest
go1.22.4 download
go1.22.4 env GOROOT

# Add this to your .zshrc
export GOROOT=/Users/pedro/sdk/go1.22.4/
export PATH=/Users/pedro/go/bin/:$PATH

sudo ln -sf /Users/pedro/sdk/go1.22.4/bin/go /usr/local/go/bin/go

curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.59.1
golangci-lint --version
MakisChristou commented 3 months ago

Here are the benchmark results before after after the changes of this PR. @libotony @otherview

bench_results_formatted_docker_before.txt bench_results_formatted_docker_after.txt

it looks like there is a slight slowdown from before the changes.

codecov-commenter commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 62.58%. Comparing base (c28bd36) to head (741aa85). Report is 2 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #775 +/- ## ========================================== + Coverage 62.50% 62.58% +0.07% ========================================== Files 199 199 Lines 18140 18152 +12 ========================================== + Hits 11338 11360 +22 + Misses 5719 5708 -11 - Partials 1083 1084 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

libotony commented 3 months ago

Here are the benchmark results before after after the changes of this PR. @libotony @otherview

bench_results_formatted_docker_before.txt bench_results_formatted_docker_after.txt

it looks like there is a slight slowdown from before the changes.

Summary for the benchmark result:

otherview commented 3 months ago

Here are the benchmark results before after after the changes of this PR. @libotony @otherview bench_results_formatted_docker_before.txt bench_results_formatted_docker_after.txt it looks like there is a slight slowdown from before the changes.

Summary for the benchmark result:

  • All _write ops is improving, but read ops are decreasing at the executing performance
  • All read/write ops are improving at the memory usage

I think the perf hit is neglatable - Also, I added https://github.com/vechain/protocol-board-repo/issues/256 where hopefully we can perhaps get some performance gains.

libotony commented 3 months ago

Here are the benchmark results before after after the changes of this PR. @libotony @otherview bench_results_formatted_docker_before.txt bench_results_formatted_docker_after.txt it looks like there is a slight slowdown from before the changes.

Summary for the benchmark result:

  • All _write ops is improving, but read ops are decreasing at the executing performance
  • All read/write ops are improving at the memory usage

I think the perf hit is neglatable - Also, I added vechain/protocol-board-repo#256 where hopefully we can perhaps get some performance gains.

Per my previous comment, the testing cases are not covering real use case, need to update the test case and check the result again.

MakisChristou commented 3 months ago

@libotony Here are the benchmark results after incorporating the changes bench_results_formatted_docker_after2.txt

libotony commented 3 months ago

@libotony Here are the benchmark results after incorporating the changes bench_results_formatted_docker_after2.txt

The change looks good, could you also post the same test on top of old source code?

libotony commented 3 months ago

@MakisChristou Looks like your test result is on arm based machine, I assume it's macOS, right? Could you also post the steps and the data used for testing? I would like to do a test in a Linux based system.

MakisChristou commented 3 months ago

@libotony Here are the benchmark results after incorporating the changes bench_results_formatted_docker_after2.txt

The change looks good, could you also post the same test on top of old source code?

Yes here are the same benchmarks on the current master branch.

bench_results_formatted_docker_before2.txt

MakisChristou commented 3 months ago

@MakisChristou Looks like your test result is on arm based machine, I assume it's macOS, right? Could you also post the steps and the data used for testing? I would like to do a test in a Linux based system.

Sure. We used the testnet_logs.db file provided by @kgapos (I think the AWS link expired so you need to reach out to him for it). The steps are the following

  1. Clone thor
  2. Choose branch (e.g. before or after the SQlite changes)
  3. Build local docker image
  4. Start docker container and map the testnet_logs.db file to the container (e.g. -v /folder/containing/testnet/logs:/home/thor/)
  5. Inside the container clone thor and run the benchmarks (e.g. go test -bench=. -benchmem -count=6 -benchtime=10s --timeout 120m github.com/vechain/thor/v2/logdb -dbPath /home/thor/testnet_logs.db)

So we are using the container that is also running thor to run the benchmarks but hopefully this is not an issue.

libotony commented 3 months ago

@MakisChristou Looks like your test result is on arm based machine, I assume it's macOS, right? Could you also post the steps and the data used for testing? I would like to do a test in a Linux based system.

Sure. We used the testnet_logs.db file provided by @kgapos (I think the AWS link expired so you need to reach out to him for it). The steps are the following

  1. Clone thor
  2. Choose branch (e.g. before or after the SQlite changes)
  3. Build local docker image
  4. Start docker container and map the testnet_logs.db file to the container (e.g. -v /folder/containing/testnet/logs:/home/thor/)
  5. Inside the container clone thor and run the benchmarks (e.g. go test -bench=. -benchmem -count=6 -benchtime=10s --timeout 120m github.com/vechain/thor/v2/logdb -dbPath /home/thor/testnet_logs.db)

So we are using the container that is also running thor to run the benchmarks but hopefully this is not an issue.

Thanks, will do a summary.

libotony commented 2 months ago

Closing in favor of #784