-
Notifications
You must be signed in to change notification settings - Fork 48
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
A fairly regular CopySlice
simplification
#366
Conversation
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.
could be a little more general maybe, but otherwise lgtm
I have now made it more general, let me know what you think! :) |
(WriteWord wOff value (ConcreteBuf buf)) dst) | ||
-- Let's not deal with overflow | ||
| n+sz >= n && n+sz >= sz = (CopySlice srcOff dstOff size | ||
(WriteWord wOff value (ConcreteBuf simplifiedBuf)) dst) |
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.
fine to merge as is, but wondering if we can have some recursive rewrites that work for buffers other than a single writeword over concretebuf?
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.
lgtm!
Description
This simplifies away some of the
ConcreteBuf
when weCopySlice
from aWriteWord
+ConcreteBuf
combo. If theConcreteBuf
is larger than what we will copy, the last parts of theConcreteBuf
is useless.Without:
With:
Notice that there was no need for all that 96B
ConcreteBuf
-- theCopySlice
will cut it out anyway. So we only keep 64B, which is whatCopySlice
will copy.Checklist