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

[tests] Speed up knapsack_solver_test by not recreating wallet 100 times. #13419

Merged
merged 1 commit into from Sep 11, 2018

Conversation

@lucash-dev
Copy link
Contributor

@lucash-dev lucash-dev commented Jun 8, 2018

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 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 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 DrahtBot commented Jun 8, 2018

No more conflicts as of last run.

@practicalswift
Copy link
Contributor

@practicalswift practicalswift commented Jun 10, 2018

Concept ACK

Moved the code for creating the wallet out of the 100-times repetition loop, for the most time-consuming tests.
@lucash-dev lucash-dev force-pushed the speedup-knapsack-solver-test branch from aa10840 to a679109 Aug 11, 2018
@lucash-dev
Copy link
Contributor Author

@lucash-dev lucash-dev commented Aug 11, 2018

Rebased.

@laanwj
Copy link
Member

@laanwj 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
1 check passed
laanwj added a commit that referenced this issue 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
@MarcoFalke
Copy link
Member

@MarcoFalke MarcoFalke commented Sep 12, 2018

@lucash-dev lucash-dev deleted the speedup-knapsack-solver-test branch Sep 15, 2018
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this issue 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 added a commit to jonspock/devault that referenced this issue 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 added a commit to devaultcrypto/devault that referenced this issue 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 added a commit to Munkybooty/dash that referenced this issue 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 added a commit to Munkybooty/dash that referenced this issue 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 added a commit to Munkybooty/dash that referenced this issue 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 added a commit to Munkybooty/dash that referenced this issue 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
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants