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

Improve the build system. #118

Closed
Determinant opened this issue Apr 30, 2020 · 12 comments
Closed

Improve the build system. #118

Determinant opened this issue Apr 30, 2020 · 12 comments
Labels
enhancement New feature or request

Comments

@Determinant
Copy link
Contributor

Determinant commented Apr 30, 2020

The Gecko project uses libraries written in other languages (crypto and network part in C/C++, for example). Thus it is non-trivial work to have a streamlined build experience as there is no official, feature-rich build system provided by golang. There are, however, some utilities (e.g. gotools) that could be utilized to help put together a customized build logic.

This post serves as an initial overview of dependencies and how packages are currently (as of Apr 30, 2020) organized in Gecko. It will subject to update in the future and the containing discussion thread will continue serving as the summary for all changes planned to the build system.

Important Dependencies

In addition to the go packages imported by Gecko's go code and resolved automatically by gotools, gecko also depends on several Go wrappers for its main functionality.

  • salticidae-go -- a Go wrapper for the salticidae library that implements the low-level p2p network messaging (in C++).
  • coreth -- a wrapper over a stock version of go-ethereum (geth) to extract the non-consensus functionalities from geth so as to support Ethereum format blocks and EVM state machine in Ava's C-chain and Athereum

Current Build Logic

We currently use a basic build script under scripts/build.sh, which:

  • first sources scripts/env.sh script to initialize some environment variables for the build:

    • GOPATH: the root prefix of all packages, including Gecko itself
    • SALTICIDAE_GO_HOME: the root directory of salticidae-go package
      The env script also checks the target system and see if the salticidae-go directory already exists (more specifically, to check if libsalticidae.a exists):
      • If it exists then just skips the process for building salticidae-go completely (this is convenient for dev, but now needs to be improved to support incremental updates for the users)
      • If not, it delegates the build of salticidae-go completely to the one-line setup script
        • Please refer to the scripts in salticidae-go for the details.
        • NOTE: for OS X, it will not build the binaries directly under the folder of salticidae-go/salticidae, but delegates this to homebrew in order to configure the cmake build environment for libsalticidae.a correctly.
  • now we assume salticidae-go is completely ready.

  • checks out the specific version of coreth

  • checks out the specific version of Gecko

  • finally build the binaries for Gecko

Principles

While the build system subjects to changes in the future, as the creator, I would humbly suggest we follow the following guidelines or at least keep them in our mind when making changes:

  • Always prioritize the experience for a novice user who may not be interested in changing the code all. This means if I'm not a programmer and just would like to try out the latest Gecko, a single command should be able to automate the whole process with defaults. Even better, there should be something like a one-liner "curl" command available for Gecko (just like the one for homebrew and rustup) that bootstraps everything from scratch ("geckoup"?).

  • The historical builds shouldn't fail: this means the build script should exactly pull the correct versions of dependencies so that in the future, even if Gecko evolves, devs are still able to successfully and easily build and reproduce older verions of it.

    • To do so, one suggestion is to keep tags for all internal dependencies (like salticidae-go and coreth) and use git tags in the build system.
  • The build system may allow upgrades. This means unless there is major change, one should easily upgrade its current Gecko clone to a specified (later) version by something like an upgrade.sh. This is very helpful for such an open-source project that gets continuously improved and so people would like to actively try out some experimental versions.

  • The behavior of build system should be documented. While this discussion thread serves as a good start, it should be somehow recorded along side the scripts.

@Determinant Determinant added the enhancement New feature or request label Apr 30, 2020
@adasari
Copy link
Contributor

adasari commented Apr 30, 2020

+1

@swdee
Copy link
Contributor

swdee commented Apr 30, 2020

You should consider the comments here #21 (comment) which when written make the build scripts unnecessary as it just integrates with Go's environment and tools.

Since that comment coreth has been established, so at most a simple Makefile could be created to build the multiple binaries; ava, evm, and xputtest.

@Determinant
Copy link
Contributor Author

I'm aware of the go module support. Yes it is related and hopefully we'll get rid of the manual parts as much as possible. But still there will be some internal build process for salticidae-go, either by Makefile or bash scripts. (Doesn't make a huge difference as salticidae already uses cmake)

@danlaine
Copy link

danlaine commented May 1, 2020

@swdee @adasari @Determinant

I've created a branch of Gecko and salticidae-go that, together, somewhat improve the build process.

The good news:

  • Uses Go modules, making dependency management (including salticidae-go and coreth) easier
  • Allows reproducible builds.
  • Makes build scripts simpler

The bad news:

  • There is still tight coupling between Gecko, salticidae-go and coreth.
  • Go modules aren't compatible with git submodules so I had to copy salticidae into salticidae-go rather than linking it
  • Untested on Mac OS X
  • Build process is, on the whole, still complicated and requires several scripts
  • Requires changing permissions on pkg/mod/salticidae-go@... which is kind of ratchet

@swdee I saw your suggestions as to how to simplify the build script. Thanks for those. I agree that it would be better to have Salticidae be installed to the OS library and to have our libraries available on an APT repo, and I would like to make those improvements in the future, but those changes require more developer time. The changes I made are an improvement, but are by no means the end of improving the build process. (Also, if we move to a different networking library this will all be moot, so there's that ;) )

Let me know what you all think, and thanks for your time.

@adasari
Copy link
Contributor

adasari commented May 1, 2020

@danlaine This is awesome beginning towards reproducible builds.

  • There is still tight coupling between Gecko, salticidae-go and coreth.

Could you please elaborate ? and name few dependencies ?

  • Go modules aren't compatible with git submodules so I had to copy salticidae into salticidae-go rather than linking it

salticidae is using C/C++. decoupling cmake may solve the problem.i never used cgo. cgo have its own challenges and platform dependencies if i understand it correctly. i will read more on this weekend.

@adasari
Copy link
Contributor

adasari commented May 1, 2020

I just read @swdee suggestion , i was referring to similar lines for salticidae with OS build parameters.

@swdee
Copy link
Contributor

swdee commented May 1, 2020

@danlaine Interesting comment of moving to a different network library, maybe a Go port of salticidae, or something like https://github.com/perlin-network/noise ?

@Determinant
Copy link
Contributor Author

Determinant commented May 2, 2020

@swdee Go is slow in dealing with large amount of short messages. Plus we don't have the leisure of time to rewrite the whole thing in Go. Please move the discussion elsewhere...

@danlaine
Copy link

danlaine commented May 5, 2020

I didn't like the approach I took before to improve the build process so I tried again
Changes are here: https://github.com/ava-labs/gecko/compare/go-mod-support-alt

The summary of the changes are:

  • Only Gecko is modified (no PR is needed to salticidae-go or salticidae)
  • Add go modules to Gecko
  • Add salticidae to Gecko's go.mod
  • Gecko's build script builds salticidae
  • The resulting library is used by Gecko
  • Allows Gecko, salticidae, salticidae-go to be updated independently

Downsides:

  • salticidae-go will not use the version of salticidae it contains as a submodule, it'll use the version of salticidae pulled by Gecko. This is kind of confusing.
  • It's kind of hacky to include salticidae in Gecko's go.mod even though it's not imported by Gecko
  • It's kind of hacky for Gecko's build script to change permissions on/build package fetched by go modules

Thoughts? @swdee @Determinant @StephenButtolph @adasari

@Determinant
Copy link
Contributor Author

Determinant commented May 6, 2020

@danlaine Thanks for the effort! I like most part of it except for this: why do we want to let Gecko pull the salticidae? The version of salticidae is highly coupled with salticidae-go, so it makes much much more sense to let salticidae-go pull the correct version of its C++ dep, while Gecko pulls in the correct version of salticidae-go. The other way around that you suggested does more harm than good IMHO.

@yilunzhang
Copy link

Go is slow in dealing with large amount of short messages.

@Determinant Just curious, what kind of speed you meant by slow here. A pure Go networking layer can easily handle hundreds of thousands of small raw messages per second (e.g. https://github.com/perlin-network/noise) or close to 100k msg/s even after protobuf marshal/unmarshal, overlay routing and encryption (e.g. https://github.com/nknorg/nnet), more than enough for tens of thousands of TPS.

StephenButtolph added a commit to StephenButtolph/avalanchego that referenced this issue Jun 24, 2020
@Determinant
Copy link
Contributor Author

Determinant commented Jun 26, 2020

@yilunzhang For your curiosity: a typical C++/Rust implementation can easily do over 400k, with hundreds of bytes (say 256) as payload. I don't see there is any value of doing a benchmark with 4 byte string as payload. Plus, apart from other abstraction overhead, GC could be the real problem. There is one thing that I might agree with you, which is go-implemented systems are generally "slow" enough so it does not demand faster messaging. Anyway, we've decided to roll with pure go network implementation. There is no value for further discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants