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

Remove Unnecessary Memory Copies with Slice to Array Conversions #23

Open
patrick-ogrady opened this issue Feb 23, 2023 · 4 comments
Open
Labels
enhancement New feature or request lifecycle/stale

Comments

@patrick-ogrady
Copy link
Contributor

patrick-ogrady commented Feb 23, 2023

go1.17 added the ability to convert from slice to array/array pointer: https://tip.golang.org/ref/spec#Conversions_from_slice_to_array_or_array_pointer

However, the UX for this was super ugly until go1.20 (just released):
image

We should now migrate all our codec unmarshaling to do this instead of copying from the tx byte slice (which will dramatically reduce memory allocations in block processing). Thanks to @rafael-abuawad for calling my attention to this in #21.

I wish we had more memory allocations benchmarked so we could show the performance improvement here but don't think it makes sense to wait to add those to add this. We can probably just add one quick one for unmarshaling a dummy tx.

We MUST be careful to only cast exactly the bytes we need so we don't keep the entire backing array of the slice alive if we don't need to (eventhough we usually will because we hold onto the tx bytes): https://utcc.utoronto.ca/~cks/space/blog/programming/GoInteriorPointerGC

@patrick-ogrady patrick-ogrady added the enhancement New feature or request label Feb 23, 2023
@patrick-ogrady patrick-ogrady added this to the v0.0.2 milestone Feb 23, 2023
@patrick-ogrady patrick-ogrady changed the title Remove Unnecessary Memory Copies w/go1.20 Remove Unnecessary Memory Copies with Slice to Array conversions Feb 23, 2023
@patrick-ogrady patrick-ogrady changed the title Remove Unnecessary Memory Copies with Slice to Array conversions Remove Unnecessary Memory Copies with Slice to Array Conversions Feb 23, 2023
@rafael-abuawad
Copy link
Contributor

rafael-abuawad commented Feb 23, 2023

#21 A optimization that I found was changing this function:

// PublicKey returns a PublicKey associated with the Ed25519 PrivateKey p.
// The PublicKey is the last 32 bytes of p.
func (p PrivateKey) PublicKey() PublicKey {
	rpk := p[32:] // privateKey == private|public
	var pk PublicKey
	copy(pk[:], rpk)
	return pk
}

With this:

// PublicKey returns a PublicKey associated with the Ed25519 PrivateKey p.
// The PublicKey is the last 32 bytes of p.
func (p PrivateKey) PublicKey() PublicKey {
	return PublicKey(p[32:])
}

@patrick-ogrady patrick-ogrady mentioned this issue Feb 25, 2023
@patrick-ogrady
Copy link
Contributor Author

This work is now unblocked!

@patrick-ogrady
Copy link
Contributor Author

This will likely involve converting a bunch of arrays to pointers to arrays so we can reuse memory from parsed blocks.

@patrick-ogrady patrick-ogrady removed this from the v0.0.2 milestone Apr 30, 2023
@github-actions
Copy link

This issue has become stale because it has been open 60 days with no activity. Adding the lifecycle/frozen label will exempt this issue from future lifecycle events.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lifecycle/stale
Projects
Archived in project
Development

No branches or pull requests

2 participants