-
Notifications
You must be signed in to change notification settings - Fork 285
Add optimizable bulk operations to Actions, native and external SetReaders #6274
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
Conversation
…-streaming-stdlibs
…-streaming-stdlibs # Conflicts: # Source/DafnyStandardLibraries/src/Std/Collections/Seq.dfy
…-streaming-stdlibs
…ing-libs # Conflicts: # Source/DafnyStandardLibraries/binaries/DafnyStandardLibraries-cs.doo # Source/DafnyStandardLibraries/binaries/DafnyStandardLibraries-go.doo # Source/DafnyStandardLibraries/binaries/DafnyStandardLibraries-java.doo # Source/DafnyStandardLibraries/binaries/DafnyStandardLibraries-js.doo # Source/DafnyStandardLibraries/binaries/DafnyStandardLibraries-notarget.doo # Source/DafnyStandardLibraries/binaries/DafnyStandardLibraries-py.doo # Source/DafnyStandardLibraries/binaries/DafnyStandardLibraries.doo
…o more-streaming-libs
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've reviewed everything, here is some general feedback and questions. Great addition !
Source/DafnyStandardLibraries/examples/TargetSpecific/ActionsExternsExamples-cs-java.dfy
Outdated
Show resolved
Hide resolved
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.
Excellent job putting everything together. Thanks for the clarifications.
What was changed?
Adds bulk operations to the Actions library -
Producer.ForEach(renamed fromForEachRemaining),Producer.Fill, andBulkAction.Map- which can use type tests to optimize for common producers and consumers. This is particularly useful when using these traits to handle binary data, especially in streaming workflows, since it allows simple modelling of operations in terms of individual byte values, but does not force actually processing individual bytes at a time (which can introduce a lot of overhead, especially in languages like Java where the type system forces boxing values intoBytewrappers).Also:
ValidChange()toValidatableand several related helper predicates/lemmas/etc, since the specification of the bulk actions heavily relied on twostate invariants of actions such asold(history) <= history.ActionsExterns.MakeSetReader<T>(s: set<T>), which creates aProducer<T>that reads the contents of a set in nondeterministic order in linear time. This is currently only implemented for Java and C#.--hidden-no-verifyoption recorded in doo files even though it is still normally unchecked, but added Makefile tests to ensure this is never used in checked-in doo files.How has this been tested?
Added lots of additional examples. Note that the bulk operations are not really tested super well, especially in the use case of interacting with external producers and consumers, but a pending change to https://github.com/smithy-lang/smithy-dafny will have substantially more coverage by using these traits to support streaming for services such as S3.
By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.