Tmpnet deferrals #429
Merged
Tmpnet deferrals #429
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
addresses review comment #401 (comment)
addresses review comment #401 (comment)
addresses review comment #401 (comment)
iansuvak
previously approved these changes
Jul 18, 2024
tests/local/network.go
Outdated
| return []interfaces.SubnetTestInfo{ | ||
| *n.subnetsInfo[n.tmpnet.GetSubnet("A").SubnetID], | ||
| *n.subnetsInfo[n.tmpnet.GetSubnet("B").SubnetID], | ||
| subnetsInfo := make([]interfaces.SubnetTestInfo, 0) |
There was a problem hiding this comment.
Performance doesn't matter much here, but when populating a new slice like this it is generally best to either:
makethe slice an initial size of 0 but an initial capacity large enough for all the entries to be added, and useappendto add the elements to it (which will not require any additional allocs)makethe slice with an initial size of the length of elements to be added, and the assign directly to the index within the for loop.
As is, it append could need to alloc a new array and copy all of the contents of the slice there on each iteration.
There was a problem hiding this comment.
It doesn't actually allocate a new array on every iteration. Slices have an underlying capacity. If you append to a full slice, golang will allocate many extra slots, which you may or may not end up using. This gives an amortized runtime cost of O(1), but it's still good practice to allocate a slice of exactly the length you need if it is known.
cam-schultz
reviewed
Jul 18, 2024
addresses review comment #429 (comment)
michaelkaplan13
previously approved these changes
Jul 19, 2024
cam-schultz
previously approved these changes
Jul 19, 2024
geoff-vball
previously approved these changes
Jul 19, 2024
Signed-off-by: F. Eugene Aumson <feuGeneA@users.noreply.github.com>
4afe45b
cam-schultz
approved these changes
Jul 19, 2024
michaelkaplan13
approved these changes
Jul 19, 2024
geoff-vball
approved these changes
Jul 19, 2024
0xchainlover
pushed a commit
to avalabsorg/teleporter
that referenced
this pull request
Aug 1, 2024
addresses review comment ava-labs/icm-contracts#429 (comment)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why this should be merged
This PR addresses some comments from the tmpnet PR that were deferred.