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

Optimize creation of funded addresses in integration tests #76

Open
msgilligan opened this issue Apr 28, 2015 · 7 comments
Open

Optimize creation of funded addresses in integration tests #76

msgilligan opened this issue Apr 28, 2015 · 7 comments

Comments

@msgilligan
Copy link
Member

Without having studied the code or its behavior too closely, it seems that many more transactions and blocks are created than would be optimal to fund a given test. As a result the integration (functional) tests take too long to run.

One thing that might be helpful in optimizing this if there were a component that could keep totals and some basic statistics on these setup transactions.

@dexX7
Copy link
Member

dexX7 commented Apr 29, 2015

The most time consuming parts are currently the "send to many owners" tests, which really slow things down, because altogether, over 1100 new addresses are created, each funded with an amount, then checked, and followed by the "send to owners" transaction. Finally balances and results are confirmed.

sto_time

The balance checks are cheap, and not many blocks are created inbetween, but the key or address generation, and sending all those transactions take a while.

A ballpark: running all regtests takes usually 2-5 minutes on my local system, which roughly matches the time spent on Jenkins, and this is very acceptable in my opinion. But it's a whole different story, when explicitly looking for lock inconsistencies, which can easily take up to an hour.

Running the STO tests with 25.000 receivers takes roughly 2 hours (without lock checks):

But let me ask: what did you have in mind, when you created this issue? As far as the Jenkins testing goes, a few minutes isn't really an issue in my opinion.

@msgilligan
Copy link
Member Author

I like to run the tests manually before doing a commit, but I'm often tempted to skip running them all and just let Jenkins run the tests. Also, when I am refactoring, I like to run the tests after sum subset of changes but before I do a commit.

As the number of tests grow, performance will become a bigger issue.

I think we could cut down on the number of blocks required to be mined if we pre-allocate BTC and MSC rather than do it in a just-in-time fashion. I don't know the current code as well as you do, so I could be wrong.

@dexX7
Copy link
Member

dexX7 commented Apr 30, 2015

You may also clear the regtest dir, but I see your point, and I see the value in the general idea here. Let me think about it.. I'm going to do some testing (and counting of operations), and report back at some point later - right now it's hard to tell for me.

@msgilligan
Copy link
Member Author

The regtest dir is cleared before the tests by both test-msc-integ-regtest.sh and test-btc-integ-regtest.sh

@dexX7
Copy link
Member

dexX7 commented May 6, 2015

I've been thinking about this, without much success.

But while creating a rather simple test for the meta DEx ("A trades x for y, B trades y for x"), which ideally runs many, many times with different values, I quickly noticed how unefficient it is, if one trade is executed after the other. It's a rather specific scenario, where no test affects the others, because new addresses and properties are created for each trade.

Nevertheless, and what I had in mind: this could probably become much more efficient with a semi-parallel pipeline, where:

  • Stage 1 creates many, many trades.
  • Stage 2 mines a block.
  • Stage 3 checks many, many balances. [...]

This introduces some overhead, because information about the actors, properties and trades should be available in the last stage.

A similar approach could be used in other scenarios as well, whether it's the transfer of tokens, creating crowdsales, or something else, as long as there are no cross-overs between the tests.

It requires a shared state object, which stores information for each test, and easy access to that information, and I was wondering, if this could be generalized.

Let's start with a simple scenario, where the second stage is mostly a placeholder:

@Stepwise
class MultiplyListElementsSpec extends Specification
{
    @Shared
    def valueList = new ArrayList<Integer>()

    @Unroll
    def "Stage 1: add element #n to a list at position #i"() {
        when:
        valueList << n
        then:
        valueList[i] == n
        where:
        n << [3, 4, 5, 6, 7]
        i << [0, 1, 2, 3, 4] // list index
    }

    def "Stage 2: multiply list elements in a single step"() {
        valueList = valueList.collect { it * 2 }
        expect:
        true // Nothing!
    }

    @Unroll
    def "Stage 3: check that the element at position #i equals #n * 2"() {
        when:
        def m = valueList[i]
        then:
        m == n * 2
        where:
        n << [3, 4, 5, 6, 7]
        i << [0, 1, 2, 3, 4] // list index
    }
}

In practise, the second would not involve multiple operations, but just one, such as mining a block.

@dexX7
Copy link
Member

dexX7 commented Jun 15, 2015

I've collected some data to get a better understanding of the situation.

  • when running all omnij-rpc tests, we currently mine 1053 blocks
  • the total time spent in "setgenerate" is roughly 45-50 seconds (of 2:00-2:10 min for all tests)
  • on average that's around 47 ms per block
  • note: block confirmations trigger transaction processing, so that's included here
  • it seems that there is no decay, and mining the 200th block is as fast as mining the 5000th block
  • mining one empty block takes on average 22 ms
  • it's slightly faster to mine several blocks a in row ("setgenerate true 30"), however this isn't useful for us*
  • the number of transactions in a block has a huge impact, and confirming the STO funding blocks can take up to 450 ms (1 coinbase transaction, 70 simple sends)

I conclude that a pipeline approach has the potential to save some time, but I believe it's not worth due to it's complexity.

Instead of trying to minimize the number of mined blocks, a much bigger impact would be to minimize the number transactions. Since this directly impacts the test quality, it's probably not practical.

However, we may look out for redundant tests.

* In one case we may save a few milliseconds:

Instead of:

// Make sure we have enough test coins
while (getBalance() < minBTCForTests) {
    // Mine blocks until we have some coins to spend
    client.generateBlocks(1)
}

... it would be better to mine several blocks at once, to get to the target value.

I've already removed this particular code part earlier, but just to demonstrate a better alternative, let's take a look at BTCTestSupport#requestBitcoin():

// Newly mined coins need to mature to be spendable
def minCoinAge = 100

if (blockCount < minCoinAge) {
    generateBlocks(minCoinAge - blockCount)
}

The only other potential improvement I can think of at the very moment is to get rid, or reduce the number of uses, of BTCTestSupport#consolidateCoins(), as this potentially involves transactions, or coin movement, which may not be necessary, and it's called after each BaseRegTestSpec.

Unfortunally I don't see a way around it, because it's there for a reason: to prevent that there are too many unspent outputs, which causes the transaction creation of Bitcoin Core to fail otherwise.

@msgilligan: what do you think? Do you see something else, which we may tackle or improve?

@dexX7
Copy link
Member

dexX7 commented Jul 11, 2015

When placing the regtest datadir on a RAM disk / tmpfs, then there is nearly a 100 % speedup, at least for me on a HDD. The gain is probably lower, when using SSDs though.

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

No branches or pull requests

2 participants