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

Implement BlockChain<T>.FindBranchPoint again, to reduce memory consumption #299

Merged
merged 3 commits into from Jul 4, 2019

Conversation

Projects
None yet
4 participants
@moreal
Copy link
Contributor

commented Jun 24, 2019

It resolves #282

Instead of using ImmutableHashSet, I choose to use IEnumerable
Because it won't load all indexes stored in the Store

@moreal moreal changed the title [WIP] Implement BlockChain<T>.FindBranchPoint again, to reduce memory consumption within [WIP] Implement BlockChain<T>.FindBranchPoint again, to reduce memory consumption Jun 24, 2019

@moreal moreal force-pushed the moreal:reduce-memory-consumption-282 branch from 017edb4 to e66c83a Jun 24, 2019

@codecov

This comment has been minimized.

Copy link

commented Jun 24, 2019

Codecov Report

Merging #299 into master will increase coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #299      +/-   ##
==========================================
+ Coverage   87.64%   87.68%   +0.04%     
==========================================
  Files         189      189              
  Lines       12768    12781      +13     
==========================================
+ Hits        11190    11207      +17     
+ Misses       1328     1324       -4     
  Partials      250      250
Impacted Files Coverage Δ
Libplanet/Blockchain/BlockChain.cs 95.69% <100%> (+0.1%) ⬆️
Libplanet/Net/Swarm.cs 77.3% <0%> (+0.31%) ⬆️

@moreal moreal force-pushed the moreal:reduce-memory-consumption-282 branch from e66c83a to 26e6a0d Jun 24, 2019

@moreal moreal changed the title [WIP] Implement BlockChain<T>.FindBranchPoint again, to reduce memory consumption Implement BlockChain<T>.FindBranchPoint again, to reduce memory consumption Jun 24, 2019

@moreal moreal force-pushed the moreal:reduce-memory-consumption-282 branch from 26e6a0d to e1bfe8c Jun 24, 2019

@dahlia

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

/rebase

@autorebase

This comment has been minimized.

Copy link
Contributor

commented Jun 25, 2019

The rebase failed:

Not Found

To rebase manually, run these commands in your terminal:

# Fetch latest updates from GitHub.
git fetch
# Create new working tree.
git worktree add .worktrees/rebase reduce-memory-consumption-282
# Navigate to the new directory.
cd .worktrees/rebase
# Rebase and resolve the likely conflicts.
git rebase --interactive --autosquash master
# Push the new branch state to GitHub.
git push --force
# Go back to the original working tree.
cd ../..
# Delete the working tree.
git worktree remove .worktrees/rebase
@dahlia

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

@moreal Could you rebase this on the current master?

@moreal moreal force-pushed the moreal:reduce-memory-consumption-282 branch from e1bfe8c to 3d12bd3 Jun 25, 2019

Show resolved Hide resolved CHANGES.md Outdated

@dahlia dahlia requested review from longfin and earlbread Jun 25, 2019

@moreal moreal force-pushed the moreal:reduce-memory-consumption-282 branch 2 times, most recently from ddce25a to 23c2223 Jun 25, 2019

Show resolved Hide resolved Libplanet/Blockchain/BlockChain.cs Outdated
Show resolved Hide resolved Libplanet/Blockchain/BlockChain.cs Outdated
Show resolved Hide resolved Libplanet/Blockchain/BlockChain.cs

@moreal moreal force-pushed the moreal:reduce-memory-consumption-282 branch 2 times, most recently from 52d6a94 to 5a330ce Jul 1, 2019

Show resolved Hide resolved CHANGES.md Outdated
Show resolved Hide resolved CHANGES.md Outdated

@moreal moreal force-pushed the moreal:reduce-memory-consumption-282 branch from 5a330ce to 100dd13 Jul 3, 2019

@dahlia
Copy link
Member

left a comment

Could you rebase this patch on the current master?

Show resolved Hide resolved CHANGES.md Outdated

@moreal moreal force-pushed the moreal:reduce-memory-consumption-282 branch from 100dd13 to a8aa4ca Jul 3, 2019

@dahlia
Copy link
Member

left a comment

Everything looks good, but you should rebase this on the current master again. 😅 An other pull request was merged few hours ago.

moreal added some commits Jun 24, 2019

Reduce memory usage in FindBranchNext
Instead of using ImmutableHashSet, I choose to use IEnumerable
Because it won't load all indexes stored in the `Store`
Add comment of `IStore.IterateIndex(string)`
Add comment of `IStore.IterateIndex(string)` to give a hint about ordering

@moreal moreal force-pushed the moreal:reduce-memory-consumption-282 branch from a8aa4ca to 63eb227 Jul 4, 2019

@dahlia

dahlia approved these changes Jul 4, 2019

@earlbread earlbread requested a review from longfin Jul 4, 2019

@longfin

longfin approved these changes Jul 4, 2019

@earlbread earlbread merged commit 8c704d8 into planetarium:master Jul 4, 2019

16 checks passed

WIP Ready for review
Details
changelog This contains self-describing changelog.
Details
codecov/patch 100% of diff hit (target 87.64%)
Details
codecov/project 87.68% (+0.04%) compared to e959dc8
Details
docs Libplanet docs generated by DocFX
Details
license/cla Contributor License Agreement is signed.
Details
planetarium.libplanet Build #20190704.5 had test failures
Details
planetarium.libplanet (Linux_Mono) Linux_Mono succeeded
Details
planetarium.libplanet (Linux_NETCore) Linux_NETCore succeeded
Details
planetarium.libplanet (Windows_Mono) Windows_Mono succeeded
Details
planetarium.libplanet (Windows_NETCore) Windows_NETCore succeeded
Details
planetarium.libplanet (Windows_NETCore_coverage) Windows_NETCore_coverage succeeded
Details
planetarium.libplanet (Windows_NETFramework) Windows_NETFramework succeeded
Details
planetarium.libplanet (macOS_Mono) macOS_Mono succeeded
Details
planetarium.libplanet (macOS_NETCore) macOS_NETCore succeeded
Details
planetarium.libplanet (macOS_Unity) macOS_Unity succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.