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

Reduce running time of fee estimation endpoint #1679

Merged
merged 1 commit into from
May 22, 2020

Conversation

rvl
Copy link
Contributor

@rvl rvl commented May 21, 2020

Issue Number

Relates to ADP-283 / #1648.

Overview

The nightly api endpoint latency benchmark got 100× slower for postTransactionFee. This fixes it by moving common setup for coin selection outside of the loop.

Comments

    Latencies for 10 fixture wallets scenario
        postTransactionFee - before - 102.2 ms
        postTransactionFee  - after - 4.8 ms

Moves the common setup outside of the loop.
@rvl rvl requested a review from piotr-iohk May 21, 2020 16:17
@rvl rvl self-assigned this May 21, 2020
@jonathanknowles jonathanknowles self-requested a review May 22, 2020 03:03
Copy link
Contributor

@jonathanknowles jonathanknowles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I've understood correctly, the general pattern seems to be:

  1. Fetch the wallet's UTxO set and also the current fee policy.
  2. Run a coin selection on the UTxO set.

Given that the estimation endpoints need to run the second phase multiple times, it seems very sensible to have this separation. 👍

@rvl
Copy link
Contributor Author

rvl commented May 22, 2020

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented May 22, 2020

@iohk-bors iohk-bors bot merged commit 8b0a6ab into master May 22, 2020
@iohk-bors iohk-bors bot deleted the rvl/adp-283/estimate-faster branch May 22, 2020 07:09
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

Successfully merging this pull request may close these issues.

2 participants