-
Notifications
You must be signed in to change notification settings - Fork 25
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
Compile-time dependencies #162
Comments
We had a similar problem with typed_ecto_schema. The fix there was to |
I am not sure whether this situation can be improved by the library. It is correct that a module compile-time-depends on the module(s) defining the types they use, because the checks are inserted as local code snippets at compile-time as well. The checks are inserted as local code snippets, which allows optimization between the code in the checks and the code in the function bodies. Ways to minimize recompilation todayThe easiest and most flexible approach is to move the type(s) you use in multiple places to a single module which effectively only contains those core types. Since the only reason for change of this module is when the types themselves change, modules depending on this module will not frequently need recompilation. Alternatively you might get some mileage --but with clear trade-offs-- from using the type overrides (then you only depend on the module with the overrides rather than the original module, however you will need to keep the types between the two in sync manually) or the automatically-extracted types (similarily you won't depend on the original module any more, but you will not be able to use the more advanced features of TypeCheck). FutureIndeed it is at least theoretically possible to move the type checking code to the original module that defines the type and re-use it. One stepping stone for allowing this feature is #132, which already is quite challenging on its own 😅. It definitely is something that would be a great improvement to the library. But it also is a large amount of work, and as such not the first thing on the list of priorities right now. (First, the current open bugs and quirks in today's usage should be fixed, and error reporting should be made configurable.) |
I am not sure I follow. Excuse my ignorance, I don't know exactly how
This code introduces a runtime dependency and not a compile-time dependency. As long as
I'm afraid this is not a viable solution for us. We've had to do this for
I understand. If you remember, please ping me if ever you get to it. I sadly won't be looking into using |
@Qqwy It depends, does the
@marcandre I thought this would also be a showstopper for us, but in practice the kind of compile-time dependencies introduced are not that bad, as in, in our case the referenced modules don't contain many (if any) runtime dependencies, and therefore we don't get any additional compile-connected dependencies, which are really bad. |
No worries! Currently, the code that is generated, contains the check inline (And as such, it needs to call functions in the original module at compile-time). This is also true for If you have, for instance, a type @type! foo :: integer() And (possibly in another module) a function with a spec:
Then at compile-time:
In other words, the code that is generated for this example function will become: def compute_a_square(val) do
case val do
x when is_integer(x) ->
result = val * val
case result do
y when is_integer(y) ->
y
_ ->
raise TypeCheck.TypeError "compute_a_square did not return a value of type `foo` etc. etc. etc."
_ ->
raise TypeCheck.TypeError "compute_a_square expected first parameter to be of type `foo` etc. etc. etc."
end (This is still slightly simplified. If you want to see the exact output, you can write The advantage here is that the BEAM is smart enough to see that e.g. we already check whether if is_integer(lhs) && is_integer(rhs) do
low_level_integer_multiplication(lhs, rhs)
else
raise ArithmeticError
end it is compiled down to only low_level_integer_multiplication(lhs, rhs) directly. |
This trickery would not be necessary if Erlang were to do cross-module code inlining. But currently Erlang does not. Moving to an approach where we do not inline the checks but instead generate code very similar in style to what you shared: def compute_a_square(val) do
RemoteModule.foo_check(val)
result = val * val
RemoteModule.foo_check(result)
result
end is something that is possible, but:
What might of course make sense, for instance, is to use the 'faster compile-time but slower runtime' variant for dev and test environments, and the 'slower compile-time but faster runtime' variant for production environments. (If checks are turned on at all for production, of course.)
Thanks a lot for sharing your concerns! Dialogue like this is very useful to improve the library. It should become something that is useful to everyone (or at least a large group) in the Elixir community. I will definitely ping you when we get around to implementing this feature! 💚 |
Thank you for the detailed explanation, makes it clear what is going on. ❤️ I sincerely do hope that the implementation will be changed to avoid this, even if the resulting code is slower. My guess is that the performance difference would be negligible, but I might be wrong; without measuring things directly it's very hard to know. |
@Qqwy That indeed makes it very clear what's going on, thanks for that! One possible approach (but it would probably require quite a major overhaul) is to use a separate mix compiler, like Domo does. Then these |
The error in #160 made me suspect that
type_check
might introduce compile-time dependencies just about everywhere.To double check this:
This compiles, but also introduces a compile-time dependency that should not be there:
I understand that
type_check
needs to generate code at compile time to check that inputs and outputs corresponds to the specified specs, but hopefully there's way for that generation to not rely on the actual spec inEmptyMix.Example
and only generate calls to that module. This would only introduce a runtime dependency onExample
and not a compile-time one.Compile-time dependencies can easily lead to many transitive dependencies and make it necessary to recompile most of a project whenever just about anything changes (see e.g. https://dashbit.co/blog/speeding-up-re-compilation-of-elixir-projects)
The text was updated successfully, but these errors were encountered: