Remove calls to heapsize #10432

cheme · 2019-02-27T13:55:07Z

This pr remove calls to heapsize crate (the crate will still be in the dependency tree from other crates).
It replace heapsize calls by malloc_size call which are basically the same but defined into 'parity-util-mem' (we got additional option and a feature for only running memory estimation (not calling the allocator)).
This cannot be merged as it is and require first that paritytech/parity-common#93 get merged and published and then that paritytech/trie#14 get merged and published. See the patched crates in Cargo.toml.

Yet it can be reviewed.

Also it is currently set (in Cargo.toml) to use jemalloc and crate size estimation (that may not be the target).

heapsize.

repo.

will require full upgrade of mem-db hash-db trie-db (memdb only is not enough). also parity common crate using those three need an upgrade (plus trie-hash need feature std to avoid error on build).

dvdplm · 2019-06-03T13:19:55Z

Note: paritytech/parity-common#93 is now merged.

dvdplm

Good stuff. I'll be back for a proper review once the trie is updated so I can run the code. :)

dvdplm · 2019-06-03T13:21:32Z

ethcore/light/src/cache.rs

-impl HeapSizeOf for Cache {
-	fn heap_size_of_children(&self) -> usize {
+
+// TODO this is fast method: should feature gate an exhaustive implementation


Please file an issue and add a ref to this comment.

dvdplm · 2019-06-03T13:22:53Z

ethcore/light/src/lib.rs

@@ -63,7 +63,8 @@ extern crate parity_bytes as bytes;
 extern crate ethereum_types;
 extern crate ethcore;
 extern crate hash_db;
-extern crate heapsize;
+extern crate parity_util_mem as malloc_size_of;
+extern crate parity_util_mem as mem;


I think this is confusing, it will require devs to come here to look. I think I'd prefer explicit paths here.

ethcore/src/engines/validator_set/test.rs

dvdplm · 2019-06-03T13:23:27Z

ethcore/src/lib.rs

@@ -97,6 +96,8 @@ extern crate patricia_trie_ethereum as ethtrie;
 extern crate rand;
 extern crate rayon;
 extern crate rlp;
+extern crate parity_util_mem as mem;
+extern crate parity_util_mem as malloc_size_of;


idem as above

dvdplm · 2019-06-03T13:24:44Z

util/journaldb/src/lib.rs

@@ -16,7 +16,8 @@

 //! `JournalDB` interface and implementation.

-extern crate heapsize;
+extern crate parity_util_mem as mem;
+extern crate parity_util_mem as malloc_size_of;


idem as above

dvdplm · 2019-06-04T10:36:53Z

Needs rebase.

dvdplm

The heapsizeof removal seems straight forward and I like it a lot.

The new trie api with all those the prefix stuff is not so nice, but I honestly don't know of a better way. :/
I'm also not clear on why we needed the prefix in the first place, but I'll ask that question elsewhere.

ethcore/evm/src/interpreter/shared_cache.rs

ethcore/light/src/client/header_chain.rs

dvdplm · 2019-06-10T09:34:32Z

@cheme I've run this branch on mainnet over the weekend. Looks good. Very odd that CI failed after last commit, seems completely unrelated. Launched a new run.

dvdplm · 2019-06-12T08:45:52Z

So things left to do here:

remove patches in Cargo.toml once all referenced crates are published
set parity-ethereum to use jemalloc
use estimation wherever possible
await needed changes to triehash before merging

…rem_heapsize

ordian

LGTM modulo David's comments.

dvdplm

This is good to go. I had some small nitpicks but if you're busy with other stuff I'm ok merging this as-is.

cheme · 2019-06-19T09:30:08Z

I am going to address them (malloc_size_of alias when it is required by the derive macro). I thought I did address them but that was not the case :(

…derive purpose)

dvdplm · 2019-06-19T11:42:34Z

@cheme merge whenever you're ready.

ethcore/sync/src/lib.rs

…anager * master: Print warnings when using dangerous settings for ValidatorSet (#10733) ethcore/res: activate atlantis classic hf on block 8772000 (#10766) refactor: Fix indentation (#10740) Updated Bn128PairingImpl to use optimized batch pairing (#10765) fix: aura don't add `SystemTime::now()` (#10720) Initialize private tx logger only if private tx functionality is enabled (#10758) Remove unused code (#10762) Remove calls to heapsize (#10432)

…dp/fix/prevent-building-block-on-top-of-same-parent * dp/chore/aura-log-validator-set-in-epoch-manager: remove dead code Treat empty account the same as non-exist accounts in EIP-1052 (#10775) docs: Update Readme with TOC, Contributor Guideline. Update Cargo package descriptions (#10652) cleanup On second thought non-validators are allowed to report Move Engine::register_client to be before other I/O handler registration (#10767) cleanup Print warnings when using dangerous settings for ValidatorSet (#10733) ethcore/res: activate atlantis classic hf on block 8772000 (#10766) refactor: Fix indentation (#10740) Updated Bn128PairingImpl to use optimized batch pairing (#10765) fix: aura don't add `SystemTime::now()` (#10720) Initialize private tx logger only if private tx functionality is enabled (#10758) Remove unused code (#10762) Remove calls to heapsize (#10432)

cheme added 16 commits December 6, 2018 20:02

Use mem utils crate from parity common

bcaa9eb

Use derive macro for clarity (even when derive impl return 0).

1776a32

Setting jemalloc as default allocator for parity client.

3a0d78d

Revert jmalloc default: need to update memorydb crate to stop using

10e30b7

heapsize.

update to new module name (alloc -> allocators), test estimate.

a159dcd

latest test

24131fd

update renamings

9157b74

test with jemalloc

1668aec

Plug onto github branch

c521348

Merge attempt for branch 'master' into rem_heapsize

e04af62

Update to reorg.

5d0b165

Merge branch 'master' into rem_heapsize

c3d23fc

switch to deprecate branch of memorydb

3f0ed81

Merge branch 'master' into rem_heapsize

f21ca1b

latest version of parity-util-mem

7c56348

Merge branch 'master' into rem_heapsize, and update to use current git

15317ca

repo.

cheme added the A0-pleasereview 🤓 Pull request needs code review. label Feb 27, 2019

soc1c added the M4-core ⛓ Core client code / Rust. label Feb 27, 2019

soc1c added this to the 2.5 milestone Feb 27, 2019

Do not switch test ...

59ab2d1

soc1c modified the milestones: 2.5, 2.6 Apr 2, 2019

In progress version switch,

32cafaa

will require full upgrade of mem-db hash-db trie-db (memdb only is not enough). also parity common crate using those three need an upgrade (plus trie-hash need feature std to avoid error on build).

dvdplm reviewed Jun 3, 2019

View reviewed changes

cheme added 4 commits June 6, 2019 11:24

in progress (need merge with memorydb update)

a571878

Branch ready to rebase (synch with latest trie changes done).

d44a8f9

Merge branch 'master' into rem_heapsize

86ef606

Remove 'estimate_heapsize' mode.

449e23a

dvdplm requested a review from ordian June 7, 2019 13:34

dvdplm reviewed Jun 8, 2019

View reviewed changes

ethcore/evm/src/interpreter/shared_cache.rs Outdated Show resolved Hide resolved

ethcore/light/src/client/header_chain.rs Outdated Show resolved Hide resolved

fix comment related to perf ignored and bitset heapsize impl.

527ff6b

cheme added 3 commits June 13, 2019 17:52

Remove patch in favor of published crates.

2653106

Merge branch 'rem_heapsize' of github.com:cheme/parity-ethereum into …

118df73

…rem_heapsize

0.12.3 trie crate were yanked, using 0.12.4

82521c4

ordian approved these changes Jun 19, 2019

View reviewed changes

Merge branch 'master' into rem_heapsize

f683856

dvdplm approved these changes Jun 19, 2019

View reviewed changes

cheme added 2 commits June 19, 2019 11:49

Refactor parity_util_mem import (crate alias are kept only for macro …

d3a5bed

…derive purpose)

Missing parity_util_mem direct usage.

620a0c1

ordian added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jun 19, 2019

ordian reviewed Jun 19, 2019

View reviewed changes

ethcore/sync/src/lib.rs Show resolved Hide resolved

cheme merged commit 6fc5014 into openethereum:master Jun 19, 2019

ordian mentioned this pull request Jun 19, 2019

Make tests pass on nightly again #9953

Closed

ordian mentioned this pull request Jul 2, 2019

unreasonably high memory usage (without crash) and won't shut down #10821

Closed

ordian mentioned this pull request Aug 6, 2019

Private contract migration and offchain state sync #10748

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove calls to heapsize #10432

Remove calls to heapsize #10432

cheme commented Feb 27, 2019

dvdplm commented Jun 3, 2019

dvdplm left a comment

dvdplm Jun 3, 2019

dvdplm Jun 3, 2019

dvdplm Jun 3, 2019

dvdplm Jun 3, 2019

dvdplm commented Jun 4, 2019

dvdplm left a comment

dvdplm commented Jun 10, 2019

dvdplm commented Jun 12, 2019 •

edited by cheme

Loading

ordian left a comment

dvdplm left a comment

cheme commented Jun 19, 2019

dvdplm commented Jun 19, 2019

Remove calls to heapsize #10432

Remove calls to heapsize #10432

Conversation

cheme commented Feb 27, 2019

dvdplm commented Jun 3, 2019

dvdplm left a comment

Choose a reason for hiding this comment

dvdplm Jun 3, 2019

Choose a reason for hiding this comment

dvdplm Jun 3, 2019

Choose a reason for hiding this comment

dvdplm Jun 3, 2019

Choose a reason for hiding this comment

dvdplm Jun 3, 2019

Choose a reason for hiding this comment

dvdplm commented Jun 4, 2019

dvdplm left a comment

Choose a reason for hiding this comment

dvdplm commented Jun 10, 2019

dvdplm commented Jun 12, 2019 • edited by cheme Loading

ordian left a comment

Choose a reason for hiding this comment

dvdplm left a comment

Choose a reason for hiding this comment

cheme commented Jun 19, 2019

dvdplm commented Jun 19, 2019

dvdplm commented Jun 12, 2019 •

edited by cheme

Loading