Skip to content

Conversation

lucash-dev
Copy link
Contributor

Optimization of knapsack_solver_testby moving an expensive wallet creation to outside a 100x for loop.

On my (slow) machine:

before:        9.8s
after:         6.2s
--------------------
saved:         3.6s (36%)

This PR was split from #13050. Also see #10026.

@fanquake fanquake added the Tests label Jun 8, 2018
@achow101
Copy link
Member

achow101 commented Jun 8, 2018

I don't think this is entirely useful considering that this test will be removed (hopefully soon) altogether by #13307.

@laanwj
Copy link
Member

laanwj commented Jun 8, 2018

I don't think this is entirely useful considering that this test will be removed (hopefully soon) altogether by #13307.

On the other hand this change is pretty straightforward, and might make sense until #13307 is merged.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jun 8, 2018

No more conflicts as of last run.

@practicalswift
Copy link
Contributor

Concept ACK

Moved the code for creating the wallet out of the 100-times repetition loop, for the most time-consuming tests.
@lucash-dev
Copy link
Contributor Author

Rebased.

@laanwj
Copy link
Member

laanwj commented Sep 11, 2018

It's months later and #13307 is still in state of flux. I'm going ahead and merging this.

@laanwj laanwj merged commit a679109 into bitcoin:master Sep 11, 2018
laanwj added a commit that referenced this pull request Sep 11, 2018
… wallet 100 times.

a679109 Speed up knapsack_solver_test by not recreating wallet 100 times. (lucash.dev@gmail.com)

Pull request description:

  Optimization of `knapsack_solver_test`by moving an expensive wallet creation to outside a 100x for loop.

  On my (slow) machine:

  ```
  before:        9.8s
  after:         6.2s
  --------------------
  saved:         3.6s (36%)
  ```

  This PR was split from #13050. Also see #10026.

Tree-SHA512: bde1a856b5f076a5845e14d1a924855c8c91742c3139b47903081289b21d01fef6f2d1fd8947058728a57de56f877bab3866af8cd1d25ba2daa44411752cdb2f
@maflcko
Copy link
Member

maflcko commented Sep 12, 2018

🎉 Congratz on halving the total memory usage of make check as well:

https://bitcoinperf.com/timeline/?exe=3%2C4%2C2%2C1&base=1%2B23&ben=makecheck.clang.3.mem-usage&env=1&revs=10&equid=on&quarts=on&extr=on

screenshot_2018-09-12 bitcoin core bench timeline

@lucash-dev lucash-dev deleted the speedup-knapsack-solver-test branch September 15, 2018 18:16
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Dec 20, 2019
… wallet 100 times.

Summary:
a679109be40491222c458fdbef58f68509dae0bd Speed up knapsack_solver_test by not recreating wallet 100 times. (lucash.dev@gmail.com)

Pull request description:

  Optimization of `knapsack_solver_test`by moving an expensive wallet creation to outside a 100x for loop.

  On my (slow) machine:

  ```
  before:        9.8s
  after:         6.2s
  --------------------
  saved:         3.6s (36%)
  ```

  This PR was split from #13050. Also see #10026.

Tree-SHA512: bde1a856b5f076a5845e14d1a924855c8c91742c3139b47903081289b21d01fef6f2d1fd8947058728a57de56f877bab3866af8cd1d25ba2daa44411752cdb2f

Backport of Core PR13419
bitcoin/bitcoin#13419

Review note: Easier to see changes if you hide white space changes (https://github.com/bitcoin/bitcoin/pull/13419/files?utf8=%E2%9C%93&diff=split&w=1)

Test Plan:
  make check
  time(./test_bitcoin --run_test=coinselector_tests/knapsack_solver_test)
Pre-patch:
  real    0m4.841s
  user    0m4.764s
  sys     0m0.076s

Post-patch:
  real    0m2.340s
  user    0m2.328s
  sys     0m0.008s

Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: Fabien, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D4760
jonspock pushed a commit to jonspock/devault that referenced this pull request Jan 10, 2020
… wallet 100 times.

Summary:
a679109be40491222c458fdbef58f68509dae0bd Speed up knapsack_solver_test by not recreating wallet 100 times. (lucash.dev@gmail.com)

Pull request description:

  Optimization of `knapsack_solver_test`by moving an expensive wallet creation to outside a 100x for loop.

  On my (slow) machine:

  ```
  before:        9.8s
  after:         6.2s
  --------------------
  saved:         3.6s (36%)
  ```

  This PR was split from #13050. Also see #10026.

Tree-SHA512: bde1a856b5f076a5845e14d1a924855c8c91742c3139b47903081289b21d01fef6f2d1fd8947058728a57de56f877bab3866af8cd1d25ba2daa44411752cdb2f

Backport of Core PR13419
bitcoin/bitcoin#13419

Review note: Easier to see changes if you hide white space changes (https://github.com/bitcoin/bitcoin/pull/13419/files?utf8=%E2%9C%93&diff=split&w=1)

Test Plan:
  make check
  time(./test_bitcoin --run_test=coinselector_tests/knapsack_solver_test)
Pre-patch:
  real    0m4.841s
  user    0m4.764s
  sys     0m0.076s

Post-patch:
  real    0m2.340s
  user    0m2.328s
  sys     0m0.008s

Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: Fabien, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D4760
proteanx pushed a commit to devaultcrypto/devault that referenced this pull request Jan 10, 2020
… wallet 100 times.

Summary:
a679109be40491222c458fdbef58f68509dae0bd Speed up knapsack_solver_test by not recreating wallet 100 times. (lucash.dev@gmail.com)

Pull request description:

  Optimization of `knapsack_solver_test`by moving an expensive wallet creation to outside a 100x for loop.

  On my (slow) machine:

  ```
  before:        9.8s
  after:         6.2s
  --------------------
  saved:         3.6s (36%)
  ```

  This PR was split from #13050. Also see #10026.

Tree-SHA512: bde1a856b5f076a5845e14d1a924855c8c91742c3139b47903081289b21d01fef6f2d1fd8947058728a57de56f877bab3866af8cd1d25ba2daa44411752cdb2f

Backport of Core PR13419
bitcoin/bitcoin#13419

Review note: Easier to see changes if you hide white space changes (https://github.com/bitcoin/bitcoin/pull/13419/files?utf8=%E2%9C%93&diff=split&w=1)

Test Plan:
  make check
  time(./test_bitcoin --run_test=coinselector_tests/knapsack_solver_test)
Pre-patch:
  real    0m4.841s
  user    0m4.764s
  sys     0m0.076s

Post-patch:
  real    0m2.340s
  user    0m2.328s
  sys     0m0.008s

Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: Fabien, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D4760
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Jul 7, 2021
…reating wallet 100 times.

a679109 Speed up knapsack_solver_test by not recreating wallet 100 times. (lucash.dev@gmail.com)

Pull request description:

  Optimization of `knapsack_solver_test`by moving an expensive wallet creation to outside a 100x for loop.

  On my (slow) machine:

  ```
  before:        9.8s
  after:         6.2s
  --------------------
  saved:         3.6s (36%)
  ```

  This PR was split from bitcoin#13050. Also see bitcoin#10026.

Tree-SHA512: bde1a856b5f076a5845e14d1a924855c8c91742c3139b47903081289b21d01fef6f2d1fd8947058728a57de56f877bab3866af8cd1d25ba2daa44411752cdb2f
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Jul 8, 2021
…reating wallet 100 times.

a679109 Speed up knapsack_solver_test by not recreating wallet 100 times. (lucash.dev@gmail.com)

Pull request description:

  Optimization of `knapsack_solver_test`by moving an expensive wallet creation to outside a 100x for loop.

  On my (slow) machine:

  ```
  before:        9.8s
  after:         6.2s
  --------------------
  saved:         3.6s (36%)
  ```

  This PR was split from bitcoin#13050. Also see bitcoin#10026.

Tree-SHA512: bde1a856b5f076a5845e14d1a924855c8c91742c3139b47903081289b21d01fef6f2d1fd8947058728a57de56f877bab3866af8cd1d25ba2daa44411752cdb2f
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Jul 9, 2021
…reating wallet 100 times.

a679109 Speed up knapsack_solver_test by not recreating wallet 100 times. (lucash.dev@gmail.com)

Pull request description:

  Optimization of `knapsack_solver_test`by moving an expensive wallet creation to outside a 100x for loop.

  On my (slow) machine:

  ```
  before:        9.8s
  after:         6.2s
  --------------------
  saved:         3.6s (36%)
  ```

  This PR was split from bitcoin#13050. Also see bitcoin#10026.

Tree-SHA512: bde1a856b5f076a5845e14d1a924855c8c91742c3139b47903081289b21d01fef6f2d1fd8947058728a57de56f877bab3866af8cd1d25ba2daa44411752cdb2f
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Jul 11, 2021
…reating wallet 100 times.

a679109 Speed up knapsack_solver_test by not recreating wallet 100 times. (lucash.dev@gmail.com)

Pull request description:

  Optimization of `knapsack_solver_test`by moving an expensive wallet creation to outside a 100x for loop.

  On my (slow) machine:

  ```
  before:        9.8s
  after:         6.2s
  --------------------
  saved:         3.6s (36%)
  ```

  This PR was split from bitcoin#13050. Also see bitcoin#10026.

Tree-SHA512: bde1a856b5f076a5845e14d1a924855c8c91742c3139b47903081289b21d01fef6f2d1fd8947058728a57de56f877bab3866af8cd1d25ba2daa44411752cdb2f
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants