Skip to content
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

GHC doesn't optimize away profunctor type classes with profiling enabled #324

Closed
minad opened this issue Jun 9, 2020 · 20 comments · Fixed by #325
Closed

GHC doesn't optimize away profunctor type classes with profiling enabled #324

minad opened this issue Jun 9, 2020 · 20 comments · Fixed by #325

Comments

@minad
Copy link

minad commented Jun 9, 2020

Hi,

I tried optics by switching over a project from lens. This worked very well due to the good type errors. I also think % is not bad and actually helps readability, since it is made clear where an optic is constructed. After finishing that I did some profiling using +RTS -p. Now Profunctor.linear and other optics-related functions appear prominently at the top of the profile. Is this some profiling artifact or is there some inlining issue present? Is the profunctor representation less efficient than the VL representation?

Another unrelated question - Many operators like +~, <>~ seem to be missing from optics? Is this intentional since those can be replicated trivially or due you generally encourange a less operator-heavy style, where not every operator is replicated for lens modifications?

Please let me know if you have better place to ask such questions! Thank you!

@arybczak
Copy link
Collaborator

arybczak commented Jun 9, 2020

Is this some profiling artifact or is there some inlining issue present? Is the profunctor representation less efficient than the VL representation?

Must be inlining issue, optics has a set of benchmarks that compare a lot of operations with lens counterparts and testsuite that checks for profunctor type classes optimizing away. Do you know where these linear calls originate from?

If you're using optics or optics-consuming functions you wrote yourself they might just need to be marked with an INLINE pragma.

Many operators like +~, <>~ seem to be missing from optics? Is this intentional since those can be replicated trivially or due you generally encourange a less operator-heavy style, where not every operator is replicated for lens modifications?

It's both reasons really.

@arybczak
Copy link
Collaborator

arybczak commented Jun 9, 2020

It might also be that profiling build doesn't optimize as well...

@minad
Copy link
Author

minad commented Jun 9, 2020

Must be inlining issue, optics has a set of benchmarks that compare a lot of operations with lens counterparts and testsuite that checks for profunctor type classes optimizing away. Do you know where these linear calls originate from?

No, I didn't check. But I straightforwardly migrated from lens - there I didn't have the problem. This means optics somehow does not inline as well.

It's both reasons really.

It is good! This gives lens a more reasonable scope. But I missed +~ a bit.

@minad
Copy link
Author

minad commented Jun 9, 2020

Did you observe something like this too, that optics-related functions appear in the profile, after migrating a non-trivial lens-heavy program? Maybe it is only a profiling optimization issue, but from time to time I like to look into the profile to detect hotspots - these will be obfuscated then.

@arybczak
Copy link
Collaborator

arybczak commented Jun 17, 2020

Did you observe something like this too, that optics-related functions appear in the profile, after migrating a non-trivial lens-heavy program?

No.

Can you share the code (or construct a testcase that demonstrates this behavior)?

A better check would be to look at linear in core output generated with -ddump-simpl -dsuppress-all -ddump-to-file.

I checked a non-trivial project and the only linear calls I can see are in the definitions generated with TH, they are optimized away at call sites (assuming that they're consumed with view or other optics function).

@minad
Copy link
Author

minad commented Jun 18, 2020

No, unfortunately I cannot share the code since it is not a small self-contained test case. But I can investigate a bit better at some point. However this has low priority for me and I am continuing to work on the lens-based branch now. I would still like to switch to optics in the future though.

@minad
Copy link
Author

minad commented Jun 18, 2020

A better check would be to look at linear in core output generated with -ddump-simpl -dsuppress-all -ddump-to-file.

I checked a non-trivial project and the only linear calls I can see are in the definitions generated with TH, they are optimized away at call sites (assuming that they're consumed with view or other optics function).

@arybczak Did you only check the resulting core output of the non-profiling or did you also check the profiling build and the profiler output? I only checked the profile, but didn't check core.

@arybczak
Copy link
Collaborator

optimized, not profiled.

@minad
Copy link
Author

minad commented Jun 18, 2020

@arybczak If you have time it would be nice if you can check a profiling build. As you wrote, it is sometimes not reporting correct results since it might impede optimizations. But still - I am using the GHC profiler quite often these days and it is very helpful in pointing out the slow parts of the application. At least much more helpful than looking at perf output, due to the flattened cps compilation model. I don't want to lose that profiling ability by switching to optics. One could argue that the ability is not really lost though, since the profiling result modulo the optics functions is still somehow valid - but I don't like this perspective. In particular since this linear function took the topmost spot in my profile. If I have more time in a while, I will certainly give optics another shot and then I can be more helpful here. I would really love to use it for its more obvious/restricted api, better errors etc. But right now, I cannot unfortunately - since I cannot develop both a lens and optics version at the same time. And any kind of performance regression due to such a switch is unacceptable, since everything lens-related should just optimize away.

@arybczak
Copy link
Collaborator

arybczak commented Jun 19, 2020

I just checked profiled build and you're right, profiled build doesn't optimize away the type classes (with GHC 8.8.3), the core is littered with calls to linear. Oh well.

I presume it's worth making a GHC issue for this. It might be intentional behavior to preserve call stacks though.

@arybczak arybczak changed the title Profiling/benchmarking question GHC doesn't optimize profunctor type classes with profiling enabled Jun 19, 2020
@arybczak arybczak changed the title GHC doesn't optimize profunctor type classes with profiling enabled GHC doesn't optimize away profunctor type classes with profiling enabled Jun 19, 2020
@arybczak
Copy link
Collaborator

arybczak commented Jun 19, 2020

Adding -flate-specialise to ghc-options fixes the problem.

It doesn't. I'm pretty sure it's intentional then.

@arybczak
Copy link
Collaborator

arybczak commented Jun 19, 2020

Here's a somewhat similar issue: https://gitlab.haskell.org/ghc/ghc/issues/12893

In particular https://gitlab.haskell.org/ghc/ghc/issues/12893#note_127879

This comment mentions how a small type class method is not inlined for some reason, preventing further optimizations from firing. Seems to be the same problem as here, where linear is not inlined, preventing further optimizations of optics definitions.

@minad
Copy link
Author

minad commented Jun 19, 2020

I would say this is not intentional or there should be at least some way to prevent this in GHC. Ideally, optimization and profiling should not interfere, but this seems to be a hard problem according to the comment by spj.

Still, I think it might make sense to open a bug report or at least ask on the GHC tracker if we manage to create a small self contained example. I think the difference here is that you are explicitly requesting inlining. In that case inlining should take precedence over good profiling data.

@arybczak
Copy link
Collaborator

arybczak commented Jun 21, 2020

I submitted a GHC ticket: https://gitlab.haskell.org/ghc/ghc/issues/18374

@arybczak
Copy link
Collaborator

arybczak commented Jun 21, 2020

@minad can you check whether #325 fixes the problem?

@minad
Copy link
Author

minad commented Jun 22, 2020

The link to the GHC issue seems dead?

@minad
Copy link
Author

minad commented Jun 22, 2020

@arybczak With the #325 fix, the situation gets better. There are still linear calls in the profile, but at least not as prominent as before.

@arybczak
Copy link
Collaborator

Ok, that's good. If we want to get this further (apart from fixing it in GHC), I'll need you to compile your project with -ddump-simpl -dsuppress-all -ddump-to-file, grep for linear in compiled core and paste the output here.

@arybczak
Copy link
Collaborator

The link to the GHC issue seems dead?

Yeah, there was a data loss during migration. The issue is at https://gitlab.haskell.org/ghc/ghc/-/issues/18374

arybczak added a commit that referenced this issue Jun 29, 2020
Optimize away profunctor classes when profiling (#324)
@arybczak
Copy link
Collaborator

Please reopen (or create a new issue) when you are able to provide core output.

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 a pull request may close this issue.

2 participants