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

rpc: speedup get_outs.bin #4821

Merged
merged 1 commit into from Nov 26, 2018

Conversation

4 participants
@moneromooo-monero
Contributor

moneromooo-monero commented Nov 7, 2018

No description provided.

@@ -1307,11 +1307,11 @@ class BlockchainDB
* get_output_data(const uint64_t& amount, const uint64_t& index)
* but for a list of outputs rather than just one.
*
* @param amount an output amount
* @param amounts an output amount, or as many as offets

This comment has been minimized.

@xiphon

xiphon Nov 7, 2018

Contributor
Suggested change Beta
* @param amounts an output amount, or as many as offets
* @param amounts an output amount, or as many as offsets

@moneromooo-monero moneromooo-monero force-pushed the moneromooo-monero:gob branch from 111119e to 6d11c1b Nov 7, 2018

offsets.push_back(i.index);
}
m_db->get_output_key(amounts, offsets, data);
if (data.size() != req.outputs.size())

This comment has been minimized.

@xiphon

xiphon Nov 7, 2018

Contributor

allow_partial is false by default, will never satisfy the condition then.

@vtnerd

What is the speedup of this patch? It doesn't seem like it would be much, considering the wallet is currently fetching 22 keys in the typical case per transaction. The patch isn't complicated, so with even a small bump, I guess its worth it.

@@ -243,7 +243,7 @@ class BlockchainLMDB : public BlockchainDB
virtual uint64_t get_num_outputs(const uint64_t& amount) const;
virtual output_data_t get_output_key(const uint64_t& amount, const uint64_t& index);
virtual void get_output_key(const uint64_t &amount, const std::vector<uint64_t> &offsets, std::vector<output_data_t> &outputs, bool allow_partial = false);
virtual void get_output_key(const std::vector<uint64_t> &amounts, const std::vector<uint64_t> &offsets, std::vector<output_data_t> &outputs, bool allow_partial = false);

This comment has been minimized.

@vtnerd

vtnerd Nov 8, 2018

Contributor

If amounts were a epee::span<const uint64_t> then the caller wouldn't have to allocate a vector for the one element case - a one element array would suffice.

Should the one element behavior of amounts be documented in the header now?

This comment has been minimized.

@moneromooo-monero

moneromooo-monero Nov 8, 2018

Contributor

Yes. It is.

@@ -50,7 +50,7 @@ namespace cryptonote
// advance which version they will stop working with
// Don't go over 32767 for any of these
#define CORE_RPC_VERSION_MAJOR 2
#define CORE_RPC_VERSION_MINOR 1
#define CORE_RPC_VERSION_MINOR 2

This comment has been minimized.

@vtnerd

vtnerd Nov 8, 2018

Contributor

What is the minor version used for? I think older clients are still compatible, as the receiver would just assume that get_txid == true (existing behavior).

This comment has been minimized.

@moneromooo-monero

moneromooo-monero Nov 8, 2018

Contributor

It gets bumped if the changes are backward compatible. They are backward compatible here as you say. A client can now tell whether some particular thing (here, get_txid) is in the daemon it's talking to.

@moneromooo-monero

This comment has been minimized.

Contributor

moneromooo-monero commented Nov 8, 2018

IIRC, when all was cached, it was about two to three times as fast. When data wasn't cached, the I/O tim was dominating anyway.

@moneromooo-monero moneromooo-monero force-pushed the moneromooo-monero:gob branch 2 times, most recently from bdce4be to ca13a94 Nov 8, 2018

@fluffypony

Reviewed

@moneromooo-monero moneromooo-monero force-pushed the moneromooo-monero:gob branch from ca13a94 to fc98f7a Nov 26, 2018

@fluffypony

Reviewed

@fluffypony fluffypony merged commit fc98f7a into monero-project:master Nov 26, 2018

1 of 9 checks passed

buildbot/monero-static-ubuntu-amd64 Build done.
Details
buildbot/monero-linux-armv7 Build started.
Details
buildbot/monero-static-osx-10.11 Build started.
Details
buildbot/monero-static-osx-10.12 Build started.
Details
buildbot/monero-static-osx-10.13 Build started.
Details
buildbot/monero-static-ubuntu-i686 Build started.
Details
buildbot/monero-static-win32 Build started.
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
buildbot/monero-static-win64 Build done.
Details

fluffypony added a commit that referenced this pull request Nov 26, 2018

Merge pull request #4821
fc98f7a rpc: speedup get_outs.bin (moneromooo-monero)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment