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

Add vl3 load balancer #744

Merged
merged 4 commits into from
Sep 14, 2023
Merged

Conversation

glazychev-art
Copy link
Contributor

@glazychev-art glazychev-art commented Sep 11, 2023

Issue: networkservicemesh/deployments-k8s#9210

It is based on CNAT VPP plugin

@glazychev-art glazychev-art marked this pull request as draft September 11, 2023 10:40
Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>
Copy link
Member

@denis-tingaikin denis-tingaikin left a comment

Choose a reason for hiding this comment

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

In general LGTM,

But I think we need to specify using of pattern serialize.Executor + map

I've tested serialize.Executor + map with go sync map tests (see at https://github.com/golang/go/blob/master/src/sync/map_bench_test.go)

package sync_test 

import (
	"fmt"
	"reflect"
	"sync"
	"sync/atomic"
	"testing"

	"github.com/edwarnicke/serialize"
)

// mapInterface is the interface Map implements.
type mapInterface interface {
	Load(any) (any, bool)
	Store(key, value any)
	Delete(any)
	Range(func(key, value any) (shouldContinue bool))
}

type bench struct {
	setup func(*testing.B, mapInterface)
	perG  func(b *testing.B, pb *testing.PB, i int, m mapInterface)
}

type MapWithExecutor struct {
	serialize.Executor
	m map[any]any
}

func (m *MapWithExecutor) Range(act func(key, value any) (shouldContinue bool)) {
	<-m.AsyncExec(func() {
		if m.m == nil {
			m.m = make(map[any]any)
		}
		for k, v := range m.m {
			if !act(k, v) {
				return
			}
		}
	})
}

func (m *MapWithExecutor) Delete(k any) {
	m.Executor.AsyncExec(func() {
		if m.m == nil {
			m.m = make(map[any]any)
		}
		delete(m.m, k)
	})
}
func (m *MapWithExecutor) Load(k any) (any, bool) {
	var r any
	var ok bool
	<-m.Executor.AsyncExec(func() {
		if m.m == nil {
			m.m = make(map[any]any)
		}
		r, ok = m.m[k]
	})
	return r, ok
}

func (m *MapWithExecutor) Store(k, v any) {
	m.Executor.AsyncExec(func() {
		if m.m == nil {
			m.m = make(map[any]any)
		}
		m.m[k] = v
	})
}

func benchMap(b *testing.B, bench bench) {
	for _, m := range [...]mapInterface{&sync.Map{}, &MapWithExecutor{}} {
		b.Run(fmt.Sprintf("%T", m), func(b *testing.B) {
			m = reflect.New(reflect.TypeOf(m).Elem()).Interface().(mapInterface)
			if bench.setup != nil {
				bench.setup(b, m)
			}

			b.ResetTimer()

			var i int64
			b.RunParallel(func(pb *testing.PB) {
				id := int(atomic.AddInt64(&i, 1) - 1)
				bench.perG(b, pb, id*b.N, m)
			})
		})
	}
}

func BenchmarkRange(b *testing.B) {
	const mapSize = 1 << 10

	benchMap(b, bench{
		setup: func(_ *testing.B, m mapInterface) {
			for i := 0; i < mapSize; i++ {
				m.Store(i, i)
			}
		},

		perG: func(b *testing.B, pb *testing.PB, i int, m mapInterface) {
			for ; pb.Next(); i++ {
				m.Range(func(_, _ any) bool { return true })
			}
		},
	})
}

// BenchmarkAdversarialDelete tests performance when we periodically delete
// one key and add a different one in a large map.
//
// This forces the Load calls to always acquire the map's mutex and periodically
// makes a full copy of the map despite changing only one entry.
func BenchmarkAdversarialDelete(b *testing.B) {
	const mapSize = 1 << 10

	benchMap(b, bench{
		setup: func(_ *testing.B, m mapInterface) {
			for i := 0; i < mapSize; i++ {
				m.Store(i, i)
			}
		},

		perG: func(b *testing.B, pb *testing.PB, i int, m mapInterface) {
			for ; pb.Next(); i++ {
				m.Load(i)

				if i%mapSize == 0 {
					m.Range(func(k, _ any) bool {
						m.Delete(k)
						return false
					})
					m.Store(i, i)
				}
			}
		},
	})
}

And I got these results:


goos: darwin
goarch: arm64
pkg: github.com/networkservicemesh/sdk/pkg/registry/chains/proxydns
BenchmarkRange
BenchmarkRange/*sync.Map
BenchmarkRange/*sync.Map-8     	   22100	     58855 ns/op	      16 B/op	       1 allocs/op
BenchmarkRange/*proxydns_test.MapWithExecutor
BenchmarkRange/*proxydns_test.MapWithExecutor-8         	   20127	     58990 ns/op	     169 B/op	       3 allocs/op
BenchmarkAdversarialDelete
BenchmarkAdversarialDelete/*sync.Map
BenchmarkAdversarialDelete/*sync.Map-8                  	 1937078	       533.8 ns/op	      25 B/op	       1 allocs/op
BenchmarkAdversarialDelete/*proxydns_test.MapWithExecutor
BenchmarkAdversarialDelete/*proxydns_test.MapWithExecutor-8         	  372580	      3555 ns/op	     242 B/op	       6 allocs/op
PASS

As you can see range + delete is 7 times faster with using sync.Map.

More over it is not using an additional goroutine (results could be more differ if we'll use more goroutine for example in prod)

So I think we should use pattern with sync.Map in most of our cases.

@glazychev-art
Copy link
Contributor Author

@denis-tingaikin
Very useful results, thanks!
Fixed, got rid of serialize.Executor

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>
pkg/tools/vl3lb/common.go Outdated Show resolved Hide resolved
Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>
Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>
@glazychev-art glazychev-art marked this pull request as ready for review September 14, 2023 12:47
@denis-tingaikin denis-tingaikin merged commit 68f99e3 into networkservicemesh:main Sep 14, 2023
13 checks passed
nsmbot pushed a commit to networkservicemesh/cmd-nsc-vpp that referenced this pull request Sep 14, 2023
…k-vpp@main

PR link: networkservicemesh/sdk-vpp#744

Commit: 68f99e3
Author: Artem Glazychev
Date: 2023-09-14 22:10:41 +0700
Message:
  - Add vl3 load balancer (#744)
* Add vl3 load balancer

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Get rid of executor

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Add logs

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Moved to chain element

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

---------

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nse-firewall-vpp that referenced this pull request Sep 14, 2023
…k-vpp@main

PR link: networkservicemesh/sdk-vpp#744

Commit: 68f99e3
Author: Artem Glazychev
Date: 2023-09-14 22:10:41 +0700
Message:
  - Add vl3 load balancer (#744)
* Add vl3 load balancer

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Get rid of executor

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Add logs

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Moved to chain element

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

---------

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nsc-simple-docker that referenced this pull request Sep 14, 2023
…k-vpp@main

PR link: networkservicemesh/sdk-vpp#744

Commit: 68f99e3
Author: Artem Glazychev
Date: 2023-09-14 22:10:41 +0700
Message:
  - Add vl3 load balancer (#744)
* Add vl3 load balancer

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Get rid of executor

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Add logs

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Moved to chain element

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

---------

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nse-icmp-responder-vpp that referenced this pull request Sep 14, 2023
…k-vpp@main

PR link: networkservicemesh/sdk-vpp#744

Commit: 68f99e3
Author: Artem Glazychev
Date: 2023-09-14 22:10:41 +0700
Message:
  - Add vl3 load balancer (#744)
* Add vl3 load balancer

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Get rid of executor

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Add logs

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Moved to chain element

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

---------

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-forwarder-vpp that referenced this pull request Sep 14, 2023
…k-vpp@main

PR link: networkservicemesh/sdk-vpp#744

Commit: 68f99e3
Author: Artem Glazychev
Date: 2023-09-14 22:10:41 +0700
Message:
  - Add vl3 load balancer (#744)
* Add vl3 load balancer

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Get rid of executor

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Add logs

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Moved to chain element

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

---------

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nse-simple-vl3-docker that referenced this pull request Sep 14, 2023
…k-vpp@main

PR link: networkservicemesh/sdk-vpp#744

Commit: 68f99e3
Author: Artem Glazychev
Date: 2023-09-14 22:10:41 +0700
Message:
  - Add vl3 load balancer (#744)
* Add vl3 load balancer

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Get rid of executor

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Add logs

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Moved to chain element

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

---------

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nse-vl3-vpp that referenced this pull request Sep 14, 2023
…k-vpp@main

PR link: networkservicemesh/sdk-vpp#744

Commit: 68f99e3
Author: Artem Glazychev
Date: 2023-09-14 22:10:41 +0700
Message:
  - Add vl3 load balancer (#744)
* Add vl3 load balancer

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Get rid of executor

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Add logs

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Moved to chain element

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

---------

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nse-vlan-vpp that referenced this pull request Sep 14, 2023
…k-vpp@main

PR link: networkservicemesh/sdk-vpp#744

Commit: 68f99e3
Author: Artem Glazychev
Date: 2023-09-14 22:10:41 +0700
Message:
  - Add vl3 load balancer (#744)
* Add vl3 load balancer

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Get rid of executor

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Add logs

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

* Moved to chain element

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>

---------

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants