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

Remove heapsize #94

Merged
merged 49 commits into from
May 31, 2019
Merged

Remove heapsize #94

merged 49 commits into from
May 31, 2019

Conversation

cheme
Copy link
Collaborator

@cheme cheme commented Jan 14, 2019

This PR is an alternative to #93 , it skips the deprecation step and directly remove heapsize usage in all parity-common crates.

Like #93 it is a follow up to #89 . It could also be a follow up to #93 if we first deprecate things.

Edit: #93 needs to be merged first because it includes fixes for macos

cheme and others added 30 commits December 6, 2018 19:41
`get_malloc_size_src.sh`.
Add missing impls.rs with ethereum types related implementation.
Fix a no_std deps issue.
Documentation enhancements.
Explicits imports.
Copy link
Member

@ordian ordian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cheme
Copy link
Collaborator Author

cheme commented Jan 16, 2019

I did resolve merge conflict, I am generally more in favor with merging #93 in a first step.
I have seen some ongoing refacto on uint and primitive crate and I fear that proceding with removal before doing the switch in parity-ethereum and substrate (I did not prepare a PR for substrate yet) may possible change on those crates for a bit.

@cheme cheme added the A1-onice label Jan 23, 2019
@cheme
Copy link
Collaborator Author

cheme commented Jan 23, 2019

I put this pr on ice (behind #93) that fix the mac behavior.

@ordian
Copy link
Member

ordian commented May 31, 2019

@cheme, could you resolve conflicts?

@ordian ordian removed the A1-onice label May 31, 2019
@ordian
Copy link
Member

ordian commented May 31, 2019

removing onice since #93 is merged

keccak-hash/Cargo.toml Outdated Show resolved Hide resolved
Co-Authored-By: Andronik Ordian <[email protected]>
Copy link
Contributor

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice in general but as this is probably going to have a huge impact upstream, we need new versions of all dependent crates.
Personally I'd love some guidance/docs in the crate on the recommended way of updating code upstream that currently use the heapsizeof crate (explicitly or implicitly through features).

transaction-pool/Cargo.toml Show resolved Hide resolved
triehash/Cargo.toml Show resolved Hide resolved
@@ -4,7 +4,6 @@ Collection of memory related utilities.

## Features

- volatile-erase : Not set by default, `Memzero` struct will be erasing memory through a simple [`write_volatile`](https://doc.rust-lang.org/std/ptr/fn.write_volatile.html) call.
- estimate-heapsize : Do not use allocator, but `size_of` or `size_of_val`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(commenting here because GH doesn't allow me to comment below)

Need to update the ## Dependency section too now that CoD is removed.

@@ -4,7 +4,6 @@ Collection of memory related utilities.

## Features
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a recommended way of using this crate to, for example, replace code that uses heapsizeof today and wishes to stop? :)

parity-util-mem/Cargo.toml Show resolved Hide resolved
kvdb-rocksdb/Cargo.toml Show resolved Hide resolved
parking_lot = { version = "*", optional = true }
elastic-array = { version = "0", optional = true }
ethereum-types = { version = "0", optional = true }
parking_lot = { version = "0", optional = true }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should work fine, says cargo publish --dry-run

@dvdplm dvdplm merged commit 8ebc5c8 into paritytech:master May 31, 2019
@dvdplm
Copy link
Contributor

dvdplm commented May 31, 2019

Published v0.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants