Skip to content

Use header size constant for faster start-ups#233

Merged
fassadlr merged 2 commits intostratisproject:masterfrom
quantumagi:fixheadersize
Nov 30, 2020
Merged

Use header size constant for faster start-ups#233
fassadlr merged 2 commits intostratisproject:masterfrom
quantumagi:fixheadersize

Conversation

@quantumagi
Copy link
Contributor

@quantumagi quantumagi commented Nov 29, 2020

The code below will at times take extremely long to run:

        public void Initialize(ChainedHeader consensusTip)
        {
            ChainedHeader current = consensusTip;

            while (current.Previous != null)
            {
                current.Previous.Next.Add(current);
                this.chainedHeadersByHash.Add(current.HashBlock, current);
                this.ChainedBlocksDataBytes += current.Header.HeaderSize;

                // TODO when pruned node is implemented it should be header only for pruned blocks
                current.BlockDataAvailability = BlockDataAvailabilityState.BlockAvailable;
                current.BlockValidationState = ValidationState.FullyValidated;

                current = current.Previous;
            }

This is due to this.ChainedBlocksDataBytes += current.Header.HeaderSize;

        public BlockHeader Header
        {
            get { return this.ChainStore.GetHeader(this, this.HashBlock); }
        }

This line of code actually reads the header from the chain store database just to access the underlying constant BlockHeader.Size:

public const int Size = 80;
...
public virtual long HeaderSize => Size;

@quantumagi quantumagi requested a review from fassadlr November 29, 2020 01:57
@quantumagi quantumagi changed the title Use header size constant Use header size constant for faster start-ups Nov 29, 2020
@fassadlr
Copy link
Contributor

@quantumagi I noticed that the HeaderSize property is overridden for proven headers:

public override long HeaderSize => Size + this.MerkleProofSize + this.SignatureSize + this.CoinstakeSize;

As we working with ProvenHeaders in STRAX, arent we always calling into ProvenBlockHeader's vesion of header size?

@quantumagi quantumagi marked this pull request as draft November 30, 2020 11:18
@quantumagi
Copy link
Contributor Author

quantumagi commented Nov 30, 2020

As we working with ProvenHeaders in STRAX, arent we always calling into ProvenBlockHeader's vesion of header size?

It's a SmartContractPoABlockHeader which derives directly from PoABlockHeader : BlockHeader with no HeaderSize override:

image

image

Notice that there is a separate member variable for proven headers.

@quantumagi quantumagi marked this pull request as ready for review November 30, 2020 12:22
@fassadlr
Copy link
Contributor

Remove the logging etc as well :)

@fassadlr fassadlr merged commit 57b3800 into stratisproject:master Nov 30, 2020
fassadlr pushed a commit that referenced this pull request Nov 30, 2020
* Use header size constant

* Remove ChainedBlocksDataBytes entirely
sondreb added a commit to block-core/blockcore that referenced this pull request May 18, 2021
- "ChainedBlocksDataBytes" was not really used, but required loading of all headers on startup. Removed to improve performance from 52 seconds to 42 seconds.
- This was discovered independent of stratisproject/StratisFullNode#233, but linking to that issue to help Stratis and Blockcore-based blockchains to take the improvement.
- #337
sondreb added a commit to block-core/blockcore that referenced this pull request May 26, 2021
- "ChainedBlocksDataBytes" was not really used, but required loading of all headers on startup. Removed to improve performance from 52 seconds to 42 seconds.
- This was discovered independent of stratisproject/StratisFullNode#233, but linking to that issue to help Stratis and Blockcore-based blockchains to take the improvement.
- #337
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