Replace flattenTrie by callback #241

Merged

bvbfan merged 2 commits from removed_flattentrie into master

2018-12-14 08:27:36 +01:00

bvbfan commented

2018-12-04 14:30:29 +01:00

(Migrated from github.com)

Signed-off-by: Anthony Fieroni bvbfan@abv.bg

Signed-off-by: Anthony Fieroni <bvbfan@abv.bg>

kaykurokawa (Migrated from github.com) reviewed 2018-12-04 14:30:29 +01:00

BrannonKing commented

2018-12-12 19:42:46 +01:00

(Migrated from github.com)

My preferred solution looks like this: 70e3f94ac8/src/Trie.hpp (L96-L181). However, I'm not quite ready to go there yet. This solution is a very safe and easy intermediate step that can be replaced later.

To clarify the intent of this: our present flattenTrie method clones the trie. Each copy of the trie uses hundreds of MB of RAM -- soon to be GB. We then clone it a second time as we copy it into the univalue objects. This PR removes the flattenTrie clone. The flattenTrie is a present part of the normalization fork one-time-upgrade that I would like to avoid. My preferred solution looks like this: https://github.com/lbryio/lbry_tries/blob/70e3f94ac8a003d3ddf67c00abc82eba1bbd4f7e/src/Trie.hpp#L96-L181. However, I'm not quite ready to go there yet. This solution is a very safe and easy intermediate step that can be replaced later.

kaykurokawa commented

2018-12-13 03:48:30 +01:00

(Migrated from github.com)

I don't really understand the "InterruptionPoint" part of the code. Can you explain a little bit how it works and what its' for? Is it based on some design pattern I can read about somewhere?

EDIT: Ok after reading a bit more.. I think I understand but it seems like we don't need this feature because we iterate through everything? Is this needed for some future use case?

I don't really understand the "InterruptionPoint" part of the code. Can you explain a little bit how it works and what its' for? Is it based on some design pattern I can read about somewhere? EDIT: Ok after reading a bit more.. I think I understand but it seems like we don't need this feature because we iterate through everything? Is this needed for some future use case?

bvbfan commented

2018-12-13 08:10:13 +01:00

(Migrated from github.com)

I think I understand but it seems like we don't need this feature because we iterate through everything?

Yes, we iterate on whole trie right now, but we can check shutdown signal in.

Is this needed for some future use case?

Yep, this approach combines features of std::for_each and std::find_if in one. It gives flexibility and performance, since we are at ~480k height iteration on whole trie is expensive.

> I think I understand but it seems like we don't need this feature because we iterate through everything? Yes, we iterate on whole trie right now, but we can check shutdown signal in. > Is this needed for some future use case? Yep, this approach combines features of std::for_each and std::find_if in one. It gives flexibility and performance, since we are at ~480k height iteration on whole trie is expensive.

No reviewers

kaykurokawa

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbrycrd#241

No description provided.