Skip to content

uint256: Add an explicit "Uninitialized" constructor for performance

Calin Culianu requested to merge cculianu/bitcoin-cash-node:base_blob_uninitialized into master

Summary

This commit adds an optional explicit uninitialized constructor to uint256 (and all base_blobs). The rationale for this is that there are many places in the code where the uint256 object is constructed for the purposes of being written-to IMMEDIATELY after construction. In such places, it makes sense to not waste CPU cycles zero-initializing the uint256 (the default c'tor always 0-initializes), but instead take an explicit fast-path that does not initialize the data blob. This is to be used only in cases where you are immediately going to fill in the entire blob with data anyway -- so zero-initializing is just a pointless waste of cycles.

Before one argues that the optimizer should take care of it: in my experiments here I found that the optimizer isn't always able to deduce that it can skip the 0-initialization of a uint256. It can only (sometimes) skip the 0-init if it "sees" all the code that fills in the uint256.. but often if the .begin() pointer is passed across translation unit boundaries (.cpp files) it cannot prove this to itself, so it does the wasteful 0-init anyway.

I intend to want to make use of this constructor in the future when optimizing code to not waste cycles. Note that the c'tor is made explicit which means you really have to want to use it and as such it is fairly safe. It is called with a type tag uint256::Uninitialized to select it -- so it should be fairly self-documenting at the call site.

I added test cases for this (as well as the default initialized c'tor) to very clearly test that it indeed does what it advertises.

I went ahead and modified random.cpp to use this explicit uint256::Uninitialized constructor in a few places to illustrate its use, to save cycles, and to give this c'tor an immediate raison d'etre.

I also then went ahead and modified uint256.h to use this in the in-line SetHex() methods. Note these methods also later explicitly clear the buffer AGAIN(!), and since they live in another translation unit (so they benefit from the optimization), so the Uninitialized optimization here is safe and recommended.

I also added a test that was missing in random_tests to test GetRandomHash().

Test Plan

  • Review code
  • ninja all check-all

No sematic changes or anything else are introduced by this commit -- this is purely an optional performance feature.

Edited by Calin Culianu

Merge request reports