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

Perf/message queue #307

Merged
merged 6 commits into from Mar 19, 2020
Merged

Perf/message queue #307

merged 6 commits into from Mar 19, 2020

Conversation

dirkmc
Copy link
Contributor

@dirkmc dirkmc commented Mar 18, 2020

Fixes #306

go test -run=XXX -benchtime=20s -bench=. -cpuprofile prof.out

Creating a new message

Old:

         .          .    493:	// Create a new message
         .      740ms    494:	msg := bsmsg.New(false)

New:

         .          .    410:	// After processing the message, clear out its fields to save memory
         .          .    411:	defer mq.msg.Reset(false)

Adding an entry to the message

Old:

         .      480ms    518:		msgSize += msg.AddEntry(e.Cid, e.Priority, wantType, false)

New:

         .      260ms    528:		msgSize += mq.msg.AddEntry(e.Cid, e.Priority, wantType, false)

Getting the wantlist

Old:

         .      1.39s    551:		for _, e := range msg.Wantlist() {

Seems a little faster, but I think things may just have moved around. In any case we're calling it once now instead of twice:
New:

         .      220ms    417:	wantlist := message.Wantlist()

@dirkmc dirkmc requested a review from Stebalien March 18, 2020 22:25
@Stebalien
Copy link
Member

#308 may be a better solution to the CID size issues.

@Stebalien
Copy link
Member

So, the race is actually in the tests. It looks like the virtual network hangs on to the message (and doesn't even bother serializing). We can probably just round-trip through a protobuf message with a bit of work.

internal/messagequeue/messagequeue.go Outdated Show resolved Hide resolved
internal/messagequeue/messagequeue.go Outdated Show resolved Hide resolved
@dirkmc
Copy link
Contributor Author

dirkmc commented Mar 19, 2020

It looks like the races are only showing up in the tests, I will go through and fix them

@dirkmc
Copy link
Contributor Author

dirkmc commented Mar 19, 2020

I updated the perf timings in the initial post at the top of this PR

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

LGTM!

@Stebalien Stebalien merged commit 89d39a6 into master Mar 19, 2020
@dirkmc dirkmc deleted the perf/message-queue branch March 19, 2020 17:00
Jorropo pushed a commit to Jorropo/go-libipfs that referenced this pull request Jan 26, 2023
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.

Bitswap gateway perf optimization
2 participants