Skip to content
This repository has been archived by the owner on Sep 23, 2023. It is now read-only.

TxPool sendersBatch optimizations #927

Merged
merged 8 commits into from
Mar 16, 2023
Merged

Conversation

Qjawko
Copy link
Contributor

@Qjawko Qjawko commented Mar 9, 2023

Reduce memory allocations and using inverted index (instead of brute-force).
Use common.Address type instead of []byte

Related issue

@AskAlexSharov
Copy link
Collaborator

@Qjawko plz create Erigon's PR (without merging this PR) to see if Erigon's CI shows green light.

@AskAlexSharov
Copy link
Collaborator

I also wanna ask you to replace:
[32]byte by common.Hash in mapset usages

txpool/pool.go Outdated

addr, ok := p.senders.senderID2Addr[metaTx.Tx.SenderID]
if !ok {
log.Warn("sender address not found by ID", metaTx.Tx.SenderID)
Copy link
Collaborator

Choose a reason for hiding this comment

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

erigon has many components - plz add prefix [txpool] flush: to understand where this log comes from

@@ -1723,7 +1725,7 @@ func (p *TxPool) fromDB(ctx context.Context, tx kv.Tx, coreTx kv.Tx) error {
txs.Resize(uint(i + 1))
txs.Txs[i] = txn
txs.IsLocal[i] = isLocalTx
copy(txs.Senders.At(i), addr)
copy(txs.Senders.At(i), addr[:])
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this line is broken - because At now returns copy of data instead of reference on underlying buffer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this line is not broken, method At still returns a pointer to array, only SliceHeader is copying (len, cap and pointer to array)

@@ -1701,7 +1703,7 @@ func (p *TxPool) fromDB(ctx context.Context, tx kv.Tx, coreTx kv.Tx) error {
if err != nil {
return err
}
addr, txRlp := v[:20], v[20:]
addr, txRlp := *(*[20]byte)(v[:20]), v[20:]
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's use BytesToAddress func
and maybe change implementation of BytesToAddress to use this "casting trick".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good advice, will try to implement

@AskAlexSharov
Copy link
Collaborator

FYI: can run on local machine:
make lintci-deps
make lint
make test

@AskAlexSharov
Copy link
Collaborator

While reviewed this task - found task for another PR: to replace mapset.NewSet by mapset.NewThreadUnsafeSet

@Qjawko
Copy link
Contributor Author

Qjawko commented Mar 10, 2023

I also wanna ask you to replace:
[32]byte by common.Hash in mapset usages

You mean change them only in txpool package, right?

@AskAlexSharov
Copy link
Collaborator

I also wanna ask you to replace:
[32]byte by common.Hash in mapset usages

You mean change them only in txpool package, right?

yes. we moved common.Hash object to erigon-lib couple weeks ago

@AskAlexSharov
Copy link
Collaborator

@Qjawko hi. next steps:

  • fix make lint
  • create PR in erigon repo which will refer to this PR - so we can see if Erigon's CI is green with this PR

then i can merge

Qjawko and others added 2 commits March 15, 2023 15:22
@AskAlexSharov AskAlexSharov added this pull request to the merge queue Mar 16, 2023
Merged via the queue into ledgerwatch:main with commit 8234bab Mar 16, 2023
@AskAlexSharov
Copy link
Collaborator

merged by: erigontech/erigon#7118

calmbeing pushed a commit to calmbeing/bsc-erigon-lib that referenced this pull request Apr 3, 2023
Reduce memory allocations and using inverted index (instead of
brute-force).
Use common.Address type instead of []byte

[Related issue](erigontech/erigon#7002)
setunapo pushed a commit to node-real/bsc-erigon-lib that referenced this pull request Apr 3, 2023
Reduce memory allocations and using inverted index (instead of
brute-force).
Use common.Address type instead of []byte

[Related issue](erigontech/erigon#7002)
calmbeing pushed a commit to calmbeing/bsc-erigon-lib that referenced this pull request Apr 24, 2023
Reduce memory allocations and using inverted index (instead of
brute-force).
Use common.Address type instead of []byte

[Related issue](erigontech/erigon#7002)
calmbeing pushed a commit to calmbeing/bsc-erigon-lib that referenced this pull request Apr 24, 2023
Reduce memory allocations and using inverted index (instead of
brute-force).
Use common.Address type instead of []byte

[Related issue](erigontech/erigon#7002)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants