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

Fix the event topic list inefficiency #371

Open
pepyakin opened this issue May 13, 2019 · 7 comments
Open

Fix the event topic list inefficiency #371

pepyakin opened this issue May 13, 2019 · 7 comments
Labels
I9-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task. T1-FRAME This PR/Issue is related to core FRAME, the framework.

Comments

@pepyakin
Copy link
Contributor

With paritytech/substrate#2491 merged, we maintain a list of topics. Each item is paired with the block number in which the item was deposited. We did that to fix the issue when a block has identical contents for two blocks, the notifications won't be triggered for the second block.

However, this scheme is clumsy, redundant and inefficient, it just was the easiest way to leverage ::append. A better solution would be to put a block number only once instead for each item.

@pepyakin pepyakin added the I9-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task. label May 13, 2019
@pepyakin
Copy link
Contributor Author

pepyakin commented May 13, 2019

Another possible solution is to introduce another map similar to EventTopics and store some value there which is dependent on the current block (i.e. the simplest - the block number) for each topic, and make light clients to watch that as the primary way of fetching updates. A light client would load the list of event indexes for the topics only if the value in that extra map were changed.

However, this would require an extra write per used topic and would make a light client to deal with one additional layer of storage indirection.

@cheme
Copy link
Contributor

cheme commented May 17, 2019

regarding paritytech/substrate#2491, I wonder why a storage double map is used for EventTopics, wouldn't a simple map be fine?

@pepyakin
Copy link
Contributor Author

I think I should’ve mentioned this in the docs: but this is the simplest way to make all storage locations to have the same prefix in tree. Thus it will allow us e.g. to use ‘kill_prefix’ there.

@cheme
Copy link
Contributor

cheme commented May 17, 2019

ok, I get it, I think we can optimize that by using kill_prefix over the internal prefix generated by the macro for a simple map (a new function may be needed)

@shawntabrizi
Copy link
Contributor

@athei Can you look at this and figure a priority?

@athei
Copy link
Member

athei commented Mar 26, 2021

The inefficiency I see here is one additional integer per event emitted. It doesn't contribute to storage bloat because this storage item is cleared in initialize. Also the process of emitting an event is dominated by the storage access. I wager that the single int does not contribute much.

I rate the priority pretty low.

@stale
Copy link

stale bot commented Jul 7, 2021

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the A5-stale label Jul 7, 2021
@pepyakin pepyakin removed the A5-stale label Jul 8, 2021
@juangirini juangirini transferred this issue from paritytech/substrate Aug 24, 2023
@the-right-joyce the-right-joyce added T1-FRAME This PR/Issue is related to core FRAME, the framework. and removed T1-runtime labels Aug 25, 2023
claravanstaden pushed a commit to Snowfork/polkadot-sdk that referenced this issue Dec 8, 2023
)

* Make Sub writer wait for Eth listener to shut down

* Improve channel use in syncer

* Make listener wait for syncer to shut down

* Test that WorkerPool stops cleanly

* Consume single payloads channel in parachain writer

* getDefaultLogger -> defaultLogger
helin6 pushed a commit to boolnetwork/polkadot-sdk that referenced this issue Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I9-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task. T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
Status: Backlog
Development

No branches or pull requests

6 participants