Skip to content

Conversation

@nsajko
Copy link
Contributor

@nsajko nsajko commented Nov 7, 2024

I don't see a justification for using @generated. The effects are good without it:

julia> Base.infer_effects(constructorof, Tuple{Type{Int}})
(+c,+e,+n,+t,+s,+m,+u,+o,+r)

Updates #21

I don't see a justification for using `@generated`. The effects are
good without it:

```julia-repl
julia> Base.infer_effects(constructorof, Tuple{Type{Int}})
(+c,+e,+n,+t,+s,+m,+u,+o,+r)
```

Updates #21
@rafaqz
Copy link
Member

rafaqz commented Nov 7, 2024

Yes, this is from before the effects system existed. (oh you mean not even using assume_effects!)

One justification is simply caution, as this function is used by a quarter of the ecosystem, and sometimes in hot inner loops.

We know the generated function will always work with every new Julia version. We hope the effects system will too, but we have no guarantee.

What is the problem the generated function is causing?

@nsajko
Copy link
Contributor Author

nsajko commented Nov 7, 2024

Ah, I just realized you still support Julia v1. I guess it's too early to merge this, then?

What is the problem the generated function is causing?

There's no specific problem, it's just good to avoid them when possible. Rule of least power comes to mind. Generated functions work on a lower level than usual functions, bypassing many parts of Julia, making them less safe.

I think getting rid of @generated could also bring compiler latency improvements, but I didn't look into it at all.

@rafaqz
Copy link
Member

rafaqz commented Nov 7, 2024

I don't know if that's a good enough reason? If there is a practical use being blocked then sure. But currently @generated is reliable, and if you look through issue history removing generated functions has caused problems in the past.

I think getting rid of @generated could also bring compiler latency improvements, but I didn't look into it at all.

I would love to see a benchmark of this. In some cases well written generated functions may be faster to compile than the alternatives. But in this PR I guess there is barely any difference?

@jw3126
Copy link
Member

jw3126 commented Nov 7, 2024

I agree with @rafaqz . There needs to be some kind of measurable benefit for getting rid of @generated here.

If there is one, happy to have an if VERSION branch without the @generated.

@nsajko nsajko closed this Nov 7, 2024
@nsajko nsajko deleted the no_generated branch November 7, 2024 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants