Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Truffle speed improvements; remove extra compilation from tests #343

Closed
1 task done
LogvinovLeon opened this issue Feb 3, 2017 · 30 comments · May be fixed by trufflesuite/truffle-core#75
Closed
1 task done

Truffle speed improvements; remove extra compilation from tests #343

LogvinovLeon opened this issue Feb 3, 2017 · 30 comments · May be fixed by trufflesuite/truffle-core#75

Comments

@LogvinovLeon
Copy link
Contributor

LogvinovLeon commented Feb 3, 2017


Issue

Truffle could be faster. We need to profile it, find bottlenecks and implement some caching.

  • truffle version takes 4s on my computer
  • truffle test with solidity tests doesn't cache compiled contracts and artifacts and rebuilds them on every run

Steps to Reproduce

  • run truffle version
  • run truffle tests with solidity tests

Expected Behavior

  • I expect time for truffle version to be negligible
  • I expect second test run to be faster than the first one and use precompiled contracts or only recompile the changed ones.
    • Maybe use a temp directory
    • Maybe have a directory inside of a project ignored in a generated .gitignore

Actual Results

Above^

Environment

  • Operating System: macos
  • Hardware: Macbook Retina 12 inch early 2016 1.1GHz Intel core m3
  • Truffle version: 3.0.5
  • Ethereum client: testrpc
  • node version: v7.5.0
  • npm version: 4.1.2
@LogvinovLeon
Copy link
Contributor Author

Talking about the first part of this issue: (Every truffle command takes at least 4 seconds)

I profiled the code and ~60% of time is spent in a C++ function which parses the module.
Here is the profiler output
It means, that the dependency tree is too big and it takes a lot of time for node to load all the modules.

Not all this code is required every time we run truffle, so the solution would be to make some of requires lazy. This will cut huge parts of dependency tree and load them on-demand.
Node made the decision, that require should not be lazy, because it was designed for writing web-serers, where startup-time is less important than latency, but if you use require inside of a function it is not executed before you actually call the function.

I propose to split by command, cause it's the most natural way to do it. We don't need to load a compiler dependencies on truffe version run.

I already tested that approach on my computer and it speeds up a lot.
For example the execution time for truffle version:

  • Before
Truffle v3.0.4

real	0m4.759s
user	0m4.112s
sys	0m0.337s
  • After
Truffle v3.0.4

real	0m0.439s
user	0m0.404s
sys	0m0.055s

Here is my diff
It just moves required inside of a run function for commands.
If you hae better ideas, I'm happy to discuss them.

I had an idea of making the file /lib/commands/index.js the splitpoint by replacing for example

compile: require("./compile"),

with

compile: () => require("./compile"),

but this is currently not possible, cause we need all the command info for yargs.

@LogvinovLeon
Copy link
Contributor Author

Talking about the second part of this issue: (Contracts are recompiled every time we run the tests)

I wrote an example implementation that caches the precompiled contracts in tests.

It's not perfect and it's a subject to discussion, but I personally am happy with that and I'm getting faster test runs.

First I used temp directory compiled_test_contracts to store compiled contracts.
The directory is in system-specific location outside of the project folder.
After implementing that I experienced an error, because Contracts.compile didn't return the paths. It has a callback with three arguments (error, abstractions, paths).
It's because in truffle-compile/index.js:118 we call the same callback with no arguments.
I tried to call that callback with an empty array of paths, but had a problem, that a second test run failed, because the dependencies were not linked.

So I just added some code for getting required deps, that does not require on paths and it started to work.

I understand, that that is not a perfect solution and I'm opened for a discussion.

@tcoulter
Copy link
Contributor

#355 has been merged.

Still looking into your solidity-test compile speed up, but it will come soon. Thanks again!

@tcoulter tcoulter changed the title Truffle speed improvements Truffle speed improvements; remove extra compilation from tests Jul 10, 2017
@zie1ony
Copy link
Contributor

zie1ony commented Aug 5, 2017

It takes up to 2s to load solc module. I made small PR: trufflesuite/truffle-compile#19 to fix it.

@zie1ony
Copy link
Contributor

zie1ony commented Aug 5, 2017

In order not to compile every time you test, make sure your build/contracts is up to date. If it's not, then truffle test will compile changed *.sol files to have latest version in /tmp/test-....

Take a look:

/code # truffle compile
Compiling ./contracts/TokenMarket.sol...
Writing artifacts to ./build/contracts

/code # truffle compile
/code # truffle test
Using network 'development'.

  Contract: TokenMarket
    ✓ should allow to check day of sale (861ms)
  1 passing (2s)

/code # echo " " >> contracts/TokenMarket.sol
/code # ./truffle test
Using network 'development'.

Compiling ./contracts/TokenMarket.sol...

  Contract: TokenMarket
    ✓ should allow to check day of sale (931ms)
  1 passing (2s)

/code # ./truffle test
Using network 'development'.

Compiling ./contracts/TokenMarket.sol...

  Contract: TokenMarket
    ✓ should allow to check day of sale (713ms)
  1 passing (2s)

/code # ./truffle compile
Compiling ./contracts/TokenMarket.sol...
Writing artifacts to ./build/contracts

/code # ./truffle test
Using network 'development'.

  Contract: TokenMarket
    ✓ should allow to check day of sale (962ms)
  1 passing (2s)

/code # 

So best way to avoid unnecessary compilation is test with: truffle compile && truffle test.

Hope that helps.

troggy added a commit to troggy/economy that referenced this issue Aug 22, 2017
Monkey patch for now. Source: trufflesuite/truffle#343
Run with 'patch node_modules/truffle/build/cli.bundled.js truffle-test-cache.patch'
troggy added a commit to troggy/economy that referenced this issue Aug 22, 2017
Monkey patch for now. Source: trufflesuite/truffle#343
Run with 'patch node_modules/truffle/build/cli.bundled.js truffle-test-cache.patch'
@troggy
Copy link
Contributor

troggy commented Aug 22, 2017

truffle compile && truffle test is not working for me. It still recompiles everything. For now, I opted to the workaround by @LogvinovLeon

johannbarbie pushed a commit to acebusters/acebusters-economy that referenced this issue Aug 23, 2017
Monkey patch for now. Source: trufflesuite/truffle#343
Run with 'patch node_modules/truffle/build/cli.bundled.js truffle-test-cache.patch'
troggy added a commit to troggy/economy that referenced this issue Sep 11, 2017
…busters#14)

Monkey patch for now. Source: trufflesuite/truffle#343
Run with 'patch node_modules/truffle/build/cli.bundled.js truffle-test-cache.patch'
@D-Nice
Copy link

D-Nice commented Oct 11, 2017

Assuming you have testrpc up, running truffle migrate --reset before any tests, avoids constant recompilation when doing several individual tests.

@Zolmeister
Copy link

Zolmeister commented Oct 17, 2017

truffle v4.0.0-beta2 patch file

301525c301525,301527
<       temp.mkdir('test-', function(err, temporaryDirectory) {
---
>       var temporaryDirectory = require('path').join(temp.dir, 'compiled_test_contracts');
>       var mkTmpDir = cb=>!fs.existsSync(temporaryDirectory) ? fs.mkdir(temporaryDirectory, cb) : cb()
>       mkTmpDir(function(err) {

@gitcoinbot
Copy link

This issue now has a funding of 2.0 ETH (655.93 USDT) attached to it. To view or claim this funding, click here.

@isaacserafino
Copy link

Question: It looks like Zolmeister indicated he fixed this a couple of hours after it was funded. But, it doesn't look like he claimed the funding. Does it no longer need fixed, or is it still available?

@Zolmeister
Copy link

@isaacserafino the PR needs to be merged - trufflesuite/truffle-core#75

@owocki
Copy link

owocki commented Nov 27, 2017

looks like the next step is for @tcoulter to do a PR review on trufflesuite/truffle-core#75 -- after that, if the team approves, i can payout the funding

@timxor
Copy link

timxor commented Dec 6, 2017

@owocki @tcoulter lmk if it bounces :D

@owocki
Copy link

owocki commented Dec 21, 2017

@Zolmeister sorry for radio ssilence.. looks like @tcoulter responded on trufflesuite/truffle-core#75

@gitcoinbot
Copy link

The funding of 2.0 ETH (1446.9 USD) attached has been claimed by @Ahmadposten.

@Ahmadposten, please leave a comment to let the funder (@owocki) and the other parties involved your implementation plan. If you don't leave a comment, the funder may expire your claim at their discretion.

@owocki
Copy link

owocki commented Dec 29, 2017

@Ahmadposten 👋 please introduce yourself. given that there's work ongoing here already... you might want to say hey to @isaacserafino and @Zolmeister et all and see if you can contribute

@Ahmadposten
Copy link

@isaacserafino @Zolmeister I found out about this issue through gitcoin looking at the issue now I can see that you submitted PRs already
Is there anything I can help with?

@isaacserafino
Copy link

@Ahmadposten I was in a similar position as you. I defer to @Zolmeister . Thank you. :)

@owocki
Copy link

owocki commented Jan 2, 2018

hey @Ahmadposten i just rejected your claim over at https://gitcoin.co/funding/details?url=https://github.com/trufflesuite/truffle/issues/343 since it looks like @Zolmeister is still lead on this. let me know if any concerns.

@Zolmeister want to claim the funds at https://gitcoin.co/funding/details?url=https://github.com/trufflesuite/truffle/issues/343 ?

ill ping the truffle folks today to see if theres any more review to be done! @benjamincburns @tcoulter i believe @Zolmeister needs a review over at trufflesuite/truffle-core#75

@vs77bb
Copy link

vs77bb commented Jan 9, 2018

@owocki @Zolmeister To check in here, looks like the issue is still listed as Open on Gitcoin. Zoli - can you claim / Kevin, is this something on our side? Happy new year, all!

@owocki
Copy link

owocki commented Jan 15, 2018

hey @Zolmeister still interested in this?

@Zolmeister
Copy link

@owocki waiting for review - trufflesuite/truffle-core#75

@owocki
Copy link

owocki commented Jan 15, 2018

@Zolmeister
Copy link

@owocki I'm afraid that might cause it to take longer. It's already been 2 months to review a 22-line change (which significantly improves user experience).

@benjamincburns
Copy link
Contributor

@Zolmeister go ahead and register your claim with the link that @owocki provided. I'm terribly sorry for the long delay on this. You clearly deserve this bounty.

@vs77bb
Copy link

vs77bb commented Jan 16, 2018

@Zolmeister Should you need gas to make the claim, DM me your MetaMask account info on slack and I can send some over :)

@gitcoinbot
Copy link

The funding of 2.0 ETH (1770.69 USD) attached has been claimed .

If you are the claimee, please leave a comment to let the funder (@owocki) and the other parties involved your implementation plan. If you don't leave a comment, the funder may expire your claim at their discretion.

@gitcoinbot
Copy link

The funding of 2.0 ETH (1770.69 USD) attached to this issue has been approved & issued .

@vs77bb
Copy link

vs77bb commented Jan 17, 2018

@Zolmeister Congrats on completion! One of the biggest outbound bounties yet. Please feel free to DM us on Slack with any suggestions about the process, we'd love to hear from you.

@mkosowsk
Copy link

big claim alert! congrats 👍 👍 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.