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

use zero copy improve string and byte convert #525

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

imxyb
Copy link
Contributor

@imxyb imxyb commented Oct 13, 2020

bench result:

BenchmarkIntForByte-12     34.3          19.8          -42.27%

benchmark                  old allocs     new allocs     delta
BenchmarkIntForByte-12     1              0              -100.00%

benchmark                  old bytes     new bytes     delta
BenchmarkIntForByte-12     3             0             -100.00%



benchmark              old ns/op     new ns/op     delta
BenchmarkString-12     14.9          2.55          -82.89%

benchmark              old allocs     new allocs     delta
BenchmarkString-12     1              0              -100.00%

benchmark              old bytes     new bytes     delta
BenchmarkString-12     4             0             -100.00%

Copy link
Collaborator

@stevenh stevenh left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, this seems like a risky change based on the comments I found here:
golang/go#25484

This refers to the fact this would be more easy when golang/go#19367 is merged so I think we should wait for that.

@stevenh
Copy link
Collaborator

stevenh commented Jul 1, 2022

Looks like the proposal golang/go#53003 has been accepted recently so we can look to use those when they are implemented.

@stevenh
Copy link
Collaborator

stevenh commented Feb 3, 2024

I believe the compiler in go 1.21 is doing this optimisation by default as there no difference between string(reply) and unsafe.String(unsafe.SliceData(reply), len(reply)) when running the following benchmark.

func BenchmarkReplyHelpers(b *testing.B) {
	c, err := dial()
	require.NoError(b, err)
	defer c.Close()

	_, err = c.Do("SET", "k1", "1")
	require.NoError(b, err)

	b.Run("String", func(b *testing.B) {
		reply, err := c.Do("GET", "k1")
		require.NoError(b, err)

		b.ResetTimer()
		for i := 0; i < b.N; i++ {
			_, err = redis.String(reply, err)
		}
		b.StopTimer()

		// Outside the loop, so the compiler cannot optimize the function call away.
		require.NoError(b, err)
	})
	b.Run("Int", func(b *testing.B) {
		reply, err := c.Do("GET", "k1")
		require.NoError(b, err)

		b.ResetTimer()
		for i := 0; i < b.N; i++ {
			_, err = redis.String(reply, err)
		}
		b.StopTimer()

		// Outside the loop, so the compiler cannot optimize the function call away.
		require.NoError(b, err)
	})
	b.Run("Int64", func(b *testing.B) {
		reply, err := c.Do("GET", "k1")
		require.NoError(b, err)

		b.ResetTimer()
		for i := 0; i < b.N; i++ {
			_, err = redis.Int64(reply, err)
		}
		b.StopTimer()

		// Outside the loop, so the compiler cannot optimize the function call away.
		require.NoError(b, err)
	})
	b.Run("Uint64", func(b *testing.B) {
		reply, err := c.Do("GET", "k1")
		require.NoError(b, err)

		b.ResetTimer()
		for i := 0; i < b.N; i++ {
			_, err = redis.Uint64(reply, err)
		}
		b.StopTimer()

		// Outside the loop, so the compiler cannot optimize the function call away.
		require.NoError(b, err)
	})
	b.Run("Float64", func(b *testing.B) {
		reply, err := c.Do("GET", "k1")
		require.NoError(b, err)

		b.ResetTimer()
		for i := 0; i < b.N; i++ {
			_, err = redis.Float64(reply, err)
		}
		b.StopTimer()

		// Outside the loop, so the compiler cannot optimize the function call away.
		require.NoError(b, err)
	})
	b.Run("Bool", func(b *testing.B) {
		reply, err := c.Do("GET", "k1")
		require.NoError(b, err)

		b.ResetTimer()
		for i := 0; i < b.N; i++ {
			_, err = redis.Bool(reply, err)
		}
		b.StopTimer()

		// Outside the loop, so the compiler cannot optimize the function call away.
		require.NoError(b, err)
	})
}

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.

2 participants