Open fwbrasil opened 6 years ago
@johnynek I think it could be three PRs:
Quoted
implicits@ttim @dieu @benpence wdyt?
IDK if it makes sense to copy, but scalatest calls something similar a Position
and may of it's methods take an implicit Position
-- seems the projection use case doesn't really fit in something called Position
-- I just worry that users are going to have no idea what a Quoted
is and if we can come up with something more clear. Is there any value in separating positional info from introspection info (eg field accesses)? if it was called MethodIntrospection
it might be more clear what it's for, not a very nice name though.
I have CallSiteInformation
in mind, but I also think it's not the best name.
@fwbrasil I think it's a good idea to split it in a way you say.
@isnotinvain @ttim Quotation is a principled concept like Monoid, Monad, etc. I think it's just a question of having documentation
@johnynek @ttim @dieu @isnotinvain I've just created the first PR: https://github.com/twitter/scalding/pull/1755
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.
Problem
Scalding could leverage more information on the user's code to apply optimizations and provide a better description of the jobs.
Solution
Introduce the new module
scalding-quotation
that provides a mechanism to quote the method parameters through an implicit parameter. Example:The quoted implicit is materialized by a macro that has access to the parameters used to invoke the method. This first version has three features:
Quoted functions
The macro has access only to the code under compilation. For instance, given this code:
The macro doesn't know the tree of
phone
, so it must assume that all fields are used and thus the projection isp.contact
instead ofp.contact.phone
. To address this limitation, the user can quote functions:Even though the macro doesn't have the projections of
phone
statically, it falls back to a runtime tree that checks if the function is quoted and determines the proper projection (p.contact.phone
in this case).Quoted propagation
The quotation must be done by the methods that are called by the user code. Any internal transformation done by scalding should use the initial quoted from the user call. For instance, this
TypedPipe
method:calls other quoted methods but it doesn't materialize new
Quoted
instances for them, reusing the one coming from the user call.As a safety measure, the macro rejects scalding sources that try to materialize a
Quoted
since that's something that shouldn't happen in scalding internally. The compilation fails with the following error if a quotation is missing and it's a scalding source file:As the error message informs, it's possible to use
Quoted.internal
as a workaround for internal transformations that don't produce projections:The macro uses a simple whitelist to allow
Quoted
materialization for source file paths that have specific substrings ("test", "example", "tutorial" for now). It's a weak rule, but it seems good enough.Enabling the automatic projection
The projection is done through an optimization phase that is disabled by default and can be enabled via the
scalding.experimental.automatic_projection_pushdown
flag. I added theexperimental
so we can promote the feature later if it's stable.Note that the automatic projection happens at an earlier phase than the manual projections configuration, so it won't have an effect if users are already using the manual projection pushdown.
New typed pipe description
The previous mechanism that provided descriptions based on stack traces will now output the information from
Quoted
instead. Example description:Interfaces like DrScalding that render the descriptions will need rework. The description could be long since the user's code can have an arbitrary length.
Backwards compatibility
All user-facing methods need to take the extra implicit Quoted. This makes the changeset binary-incompatible, but it's mostly source-compatible. It'll not compile only if the user has a method call that specifies implicits explicitly. Example:
Notes
.gitignore
. It's just a convenience for me; I can remove if you prefer.transitiveDependentsOf
made me realize how convenient it is :)