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

Save last db read #7056

Merged
merged 2 commits into from Jan 22, 2016

Conversation

Projects
None yet
5 participants
@morcos
Member

morcos commented Nov 18, 2015

It's possible coins with the same hash exist when you create a duplicate coinbase, so previously we were reading from the database to make sure we had the old coins cached so if we were to spend the new ones, the old ones would also be spent. This pull instead just marks the new coins as not fresh if they are from a coinbase, so if they are spent they will be written all the way down to the database anyway overwriting any duplicates.

This requires #5967 in order to work.

morcos added some commits Mar 26, 2015

Alter assumptions in CCoinsViewCache::BatchWrite
Previously it would break if you flushed a parent cache while there was a child cache referring to it.  This change will allow the flushing of parent caches.
Save the last unnecessary database read
It's possible coins with the same hash exist when you create a duplicate coinbase, so previously we were reading from the database to make sure we had the old coins cached so if we were to spend the new ones, the old ones would also be spent.  This pull instead just marks the new coins as not fresh if they are from a coinbase, so if they are spent they will be written all the way down to the database anyway overwriting any duplicates.
@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Nov 28, 2015

Member

utACK

Member

sipa commented Nov 28, 2015

utACK

@btcdrak

This comment has been minimized.

Show comment
Hide comment
@btcdrak

btcdrak Jan 20, 2016

Member

#5967 was merged

utACK

Member

btcdrak commented Jan 20, 2016

#5967 was merged

utACK

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Jan 21, 2016

Member

Heh, I was convinced we merged this... I guess it'll have to wait for 0.13 now.

Member

sipa commented Jan 21, 2016

Heh, I was convinced we merged this... I guess it'll have to wait for 0.13 now.

@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Jan 21, 2016

Contributor

@sipa does it?
@morcos what is the motivation for this PR? Is it a bug fix? A performance improvement?
If the intentions are clear, it may be able to make it for 0.13.

We're only feature frozen right?

Contributor

dcousens commented Jan 21, 2016

@sipa does it?
@morcos what is the motivation for this PR? Is it a bug fix? A performance improvement?
If the intentions are clear, it may be able to make it for 0.13.

We're only feature frozen right?

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Jan 21, 2016

Member

Performance improvement, and only that. Certainly nothing that should go in 0.12 now.

Member

sipa commented Jan 21, 2016

Performance improvement, and only that. Certainly nothing that should go in 0.12 now.

@laanwj laanwj merged commit 8504867 into bitcoin:master Jan 22, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Jan 22, 2016

Merge #7056: Save last db read
8504867 Save the last unnecessary database read (Alex Morcos)
@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Jan 22, 2016

Contributor

So I guess that means we'll have this for 0.12 @laanwj?
I'll start testing with it in any case.

Contributor

dcousens commented Jan 22, 2016

So I guess that means we'll have this for 0.12 @laanwj?
I'll start testing with it in any case.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jan 23, 2016

Member

@dcousens No, I'm not going to backport it, there's no urgent need, this is fine for 0.13 which means it can be implicitly tested for 6 months in master

Member

laanwj commented Jan 23, 2016

@dcousens No, I'm not going to backport it, there's no urgent need, this is fine for 0.13 which means it can be implicitly tested for 6 months in master

@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Jan 23, 2016

Contributor

No worries. I wasn't sure if master was still for 0.12 or not 👍

Contributor

dcousens commented Jan 23, 2016

No worries. I wasn't sure if master was still for 0.12 or not 👍

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge #7056: Save last db read
8504867 Save the last unnecessary database read (Alex Morcos)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #7056: Save last db read
8504867 Save the last unnecessary database read (Alex Morcos)

codablock added a commit to codablock/dash that referenced this pull request Sep 27, 2017

Merge #7056: Save last db read
8504867 Save the last unnecessary database read (Alex Morcos)

codablock added a commit to codablock/dash that referenced this pull request Oct 12, 2017

Merge #7056: Save last db read
8504867 Save the last unnecessary database read (Alex Morcos)

codablock added a commit to codablock/dash that referenced this pull request Oct 19, 2017

Merge #7056: Save last db read
8504867 Save the last unnecessary database read (Alex Morcos)

UdjinM6 added a commit to UdjinM6/dash that referenced this pull request Nov 8, 2017

Merge #7056: Save last db read
8504867 Save the last unnecessary database read (Alex Morcos)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment