-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Updated KeyBuilder
Fast Access
#3768
base: master
Are you sure you want to change the base?
Updated KeyBuilder
Fast Access
#3768
Conversation
Any benchmark data? |
_stream.WriteByte(prefix); | ||
private void CheckLength(int length) | ||
{ | ||
if ((length + _keyLength) > _cacheData.Length) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Increase it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why? Is not >=
or what do you mean
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, increase the array instead of throw an error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the Length is passed in, we need to enforce that length. 99% of the time we know the length up front?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise is not a hint, and is not working as before, it should not happen so it won't have any impact in the performance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had @superboyiii test this on mainnet and testnet, no key is bigger than 64 bytes. So we need to enforce. If you want bigger key you must know the size before hand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It haven't got any sense to me, throw an exception when is possible to increase the buffer, usability vs optimization? Could you clarify why we should not increase the buffer and preserve the previous behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already did state that. You pass in a LENGTH. What's the point of doing that if it not enforced? Im sick of everything being programmed around these bugs. It needs to stop. Have some discipline.
Co-authored-by: nan01ab <[email protected]>
Description
Fixed with Disposing
MemoryStream
onToArray()
. Made a lot faster as well. Enforced Overflows and other lengths.Type of change
How Has This Been Tested?
Checklist: