well-typed / optics

Optics as an abstract interface
374 stars 24 forks source link

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

Closed minad closed 4 years ago

minad commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

minad commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

optimized, not profiled.

minad commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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

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

arybczak commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

arybczak commented 4 years ago

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

minad commented 4 years ago

The link to the GHC issue seems dead?

minad commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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