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.

Sign up for GitHub

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

chore: replace unbuffered channels with buffered channels #1668

Merged
merged 14 commits into from
Jul 9, 2021
Merged

chore: replace unbuffered channels with buffered channels #1668

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
6885bae
add buffer to channels
edwardmack Jul 1, 2021
797b0bf
add buffers to channels in tests
edwardmack Jul 1, 2021
0bbd004
Merge branch 'development' into ed/add_channel_buffers
edwardmack Jul 2, 2021
c84976a
remove buffers from channels that shouldn't be buffered
edwardmack Jul 2, 2021
634050b
Merge branch 'development' into ed/add_channel_buffers
edwardmack Jul 2, 2021
8ed5ee6
Merge branch 'development' into ed/add_channel_buffers
edwardmack Jul 6, 2021
39eb00f
added DEFAULT_BUFFERS_SIZE const
edwardmack Jul 6, 2021
504e4f6
Merge branch 'development' into ed/add_channel_buffers
edwardmack Jul 7, 2021
6395b5c
lint
edwardmack Jul 7, 2021
79db38b
Merge branch 'development' into ed/add_channel_buffers
edwardmack Jul 7, 2021
c8936e5
Merge branch 'development' into ed/add_channel_buffers
edwardmack Jul 7, 2021
3e43488
addres comments
edwardmack Jul 8, 2021
5cd262c
Merge branch 'development' into ed/add_channel_buffers
edwardmack Jul 8, 2021
651e07a
Merge branch 'development' into ed/add_channel_buffers
edwardmack Jul 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions dot/core/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ import (
"github.com/ChainSafe/gossamer/lib/transaction"
)

// DEFAULT_BUFFER_SIZE buffer size for channels
const DEFAULT_BUFFER_SIZE = 100
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe move this to common or dot/rpc/subscription or such since core isn't using it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to dot/rpc/subscription


// BlockState interface for block state methods
type BlockState interface {
BestBlockHash() common.Hash
Expand Down
8 changes: 4 additions & 4 deletions dot/rpc/subscription/listeners_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestStorageObserver_Update(t *testing.T) {
}

func TestBlockListener_Listen(t *testing.T) {
notifyChan := make(chan *types.Block)
notifyChan := make(chan *types.Block, 100)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make the value 100 constant and all packages should use it?
It will make it easy to configure.

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've added const DEFAULT_BUFFER_SIZE, and removed the buffers from the channels that are used in tests.

Copy link
Member

Choose a reason for hiding this comment

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

This one could be unbuffered?

mockConnection := &MockWSConnAPI{}
bl := BlockListener{
Channel: notifyChan,
Expand All @@ -95,7 +95,7 @@ func TestBlockListener_Listen(t *testing.T) {
}

func TestBlockFinalizedListener_Listen(t *testing.T) {
notifyChan := make(chan *types.FinalisationInfo)
notifyChan := make(chan *types.FinalisationInfo, 100)
Copy link
Member

Choose a reason for hiding this comment

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

could be unbuffered?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

mockConnection := &MockWSConnAPI{}
bfl := BlockFinalizedListener{
channel: notifyChan,
Expand All @@ -121,8 +121,8 @@ func TestBlockFinalizedListener_Listen(t *testing.T) {
}

func TestExtrinsicSubmitListener_Listen(t *testing.T) {
notifyImportedChan := make(chan *types.Block)
notifyFinalizedChan := make(chan *types.FinalisationInfo)
notifyImportedChan := make(chan *types.Block, 100)
notifyFinalizedChan := make(chan *types.FinalisationInfo, 100)

mockConnection := &MockWSConnAPI{}
esl := ExtrinsicSubmitListener{
Expand Down
9 changes: 5 additions & 4 deletions dot/rpc/subscription/websocket.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"strings"
"sync"

"github.com/ChainSafe/gossamer/dot/core"
"github.com/ChainSafe/gossamer/dot/rpc/modules"
"github.com/ChainSafe/gossamer/dot/state"
"github.com/ChainSafe/gossamer/dot/types"
Expand Down Expand Up @@ -228,7 +229,7 @@ func (c *WSConn) unsubscribeStorageListener(reqID float64, l Listener, _ interfa

func (c *WSConn) initBlockListener(reqID float64, _ interface{}) (Listener, error) {
bl := &BlockListener{
Channel: make(chan *types.Block),
Channel: make(chan *types.Block, core.DEFAULT_BUFFER_SIZE),
wsconn: c,
}

Expand Down Expand Up @@ -260,7 +261,7 @@ func (c *WSConn) initBlockListener(reqID float64, _ interface{}) (Listener, erro

func (c *WSConn) initBlockFinalizedListener(reqID float64, _ interface{}) (Listener, error) {
bfl := &BlockFinalizedListener{
channel: make(chan *types.FinalisationInfo),
channel: make(chan *types.FinalisationInfo, 100),
Copy link
Member

Choose a reason for hiding this comment

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

could use core.DEFAULT_BUFFER_SIZE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, updated.

wsconn: c,
}

Expand Down Expand Up @@ -299,10 +300,10 @@ func (c *WSConn) initExtrinsicWatch(reqID float64, params interface{}) (Listener

// listen for built blocks
esl := &ExtrinsicSubmitListener{
importedChan: make(chan *types.Block),
importedChan: make(chan *types.Block, core.DEFAULT_BUFFER_SIZE),
wsconn: c,
extrinsic: types.Extrinsic(extBytes),
finalisedChan: make(chan *types.FinalisationInfo),
finalisedChan: make(chan *types.FinalisationInfo, 100),
Copy link
Member

Choose a reason for hiding this comment

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

could use core.DEFAULT_BUFFER_SIZE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

}

if c.BlockAPI == nil {
Expand Down