search

paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.network/
1.86k stars 680 forks source link

Responsible Memory Usage #562

Open gavofyork opened 4 years ago

gavofyork commented 4 years ago

News: https://hackmd.io/pvj8fsC3QbSc7o54ZH4qJQ

There are memory leaks in the codebase. Not only do we need to fix them, we need a proper attitude to ensure that they do not happen again.

Thus we need to achieve three things:

  1. A solid and well-documented approach to heap tracing.

  2. To rewrite our optional allocations under an attitude of zero-cost optimisations and, in general, ensure that all dynamic allocations are bounded.

  3. To integrate all dynamic allocations into our triad of reporting apparatus: prometheus, telemetry and informant.

Heap tracing

It should not take days of research to figure out how to determine what lines in the codebase are resulting in so much memory being allocated. We need a clear, well-maintained document on how to setup and run Substrate (and derivative clients like Polkadot) in order to get full traces of allocations.

There are several tools that may be used (heaptrack, jemalloc, Massif/Valgrind, ...) and it is not clear which are viable, and/or sensible.

Zero-cost optimisations

This is principle a little like the "pay for what you use" mantra of Zero-Cost Abstractions that Rust gives. Essentially, it should be possible to "turn off" or reduce non-mandatory memory allocations to a large degree. This would come at a potentially large performance cost, but we will assume for now that under some circumstances this is acceptable, and in any case, having the ability to tune how much memory is allowed to be allocated in each of the various dynamic subsystems is useful.

We need to introduce CLI options such that, at the extreme end, a user (or, developer) can minimise expected memory usage. In many cases, this memory usage (given by buffer sizes, cache limits, queue capacities and pre-allocation values) is hard-coded. In other cases, it's unbounded. It should, at all times, be bounded whose value is given by the user to be configured over the CLI.

It should be configurable for all dynamically allocating subsystems. Every buffer, queue and cache. Every time something it allocated and kept around for later use. Every allocation that isn't strictly required for normal operation. If a buffer or queue is automatically grown under some circumstance, it should be instantly shrunk again once the need is gone.

One example would be the 1GB that is given to rocks db's cache size - it should be possible to lower this, all the way to zero, ideally. There are, I'm sure, many examples.

Items that allocate large amounts but that are used transiently (such the memory slab for wasm execution) should be configurable so that they remain allocated only as long as they are being actively used. This will mean that every execution will need to allocate afresh, but we don't care.

The end result of this effort is to be able to lower the expected amount of memory used in stable-state (i.e. when not importing or authoring) to around 250 MB, even with dozens of peers.

Footprint reporting

In addition to this, every dynamically allocated queue, buffer and cache should be continuously reporting its size and footprint out to telemetry, informant and/or prometheus. We should obviously begin with the lower-hanging fruit - those that we suspect of being bad behavers, but eventually we should have an attitude of reporting everything.

We probably want to do this only in a certain mode of operation, to avoid performance costs under normal operation.

Related: paritytech/substrate#4679

burdges commented 4 years ago

Anyone look into our Reed-Solomn crate? https://github.com/darrenldl/reed-solomon-erasure/issues/74

gnunicorn commented 4 years ago

@burdges could you elaborate? I can't find that crate in our dependency tree.

burdges commented 4 years ago

We use the fork https://github.com/paritytech/reed-solomon-erasure that maybe we should upstream, etc.

darkfriend77 commented 3 years ago

what are the tools suggested to investigate the bad behaviour in v3.0.0 concerning memory?

https://github.com/paritytech/substrate/issues/8117

heaptrack didn't worked out for me here

ghost commented 1 year ago

paritytech deleted a comment from abebeos 4 days ago

Thought this is an open project. Who deletes valid comments in such an nontransparent manner?

No one should have this right. Solana outages happen because of such arrogant behavior, especially by mvines (https://github.com/systemic-flaws/dapp-platforms/issues/5).

Who is it here? Show yourself!

(I assume that the label was not removed after reaching/closing of milestone 2.0: "gavofyork added this to the 2.0 milestone [on Mar 2, 2020]"(https://github.com/paritytech/polkadot-sdk/issues/562) - and if you look at this milestone, you notice 2 wrongly assigned issues )

bkchr commented 1 year ago

Thought this is an open project. Who deletes valid comments in such an nontransparent manner?

No one should have this right. Solana outages happen because of such arrogant behavior, especially by mvines (systemic-flaws/dapp-platforms#5).

Just going around spamming issues is a reason to have comments removed. You can go around and you will find from time to time comments that have been removed. There are no comments being removed that are about the topic of the issue. Sorry if that is against your believes.

ghost commented 1 year ago

I do not care about "spam issue removal".

I care about my comment that was removed, completely unjustified.

Sorry if that is against your believes.

This is nothing about my believes, but about Substrate/Paritytech own requirements/statements re "project openness".

Who deleted my (valid, in-topic) comment here?

ghost commented 1 year ago

There are no comments being removed that are about the topic of the issue.

Yes, there are: my comment. It was valid (comments re labeling and other meta-data is naturally in-topic)

bkchr commented 1 year ago

At the point you commented on multiple issues, which looked like spam.

ghost commented 1 year ago

At the point you commented on multiple issues, which looked like spam.

Wait! Was it you, in "no coffee yet" mode? Or do you have an automated bot ~which~ that shreds based on comment-count?

ghost commented 1 year ago

(funny thing: the real spam survives, see https://github.com/paritytech/substrate/issues/4728#issuecomment-1445398133. Even my report to github support had no effect...)

ghost commented 1 year ago

(just for the sake of completeness)

@gavofyork , this is actually an issue suitable for a bounty:

Place such a simple(!) bounty (without essays about processing etc., it is a standard-problem), and you should have a result soon (instead of having the issue open for further 2 years).

Triage

Needs to be removed from https://github.com/paritytech/substrate/milestone/10 (closed milestone should not have issues assigned)