search

well-typed / optics

Optics as an abstract interface
374 stars 24 forks source link

Consider reducing the amount of Field* classes in Data.Tuple.Optics #110

Closed arybczak closed 5 years ago

arybczak commented 5 years ago

Currently Data.Tuple.Optics module is huge and it takes a lot of time to compile it. I wonder how useful supporting 19 fields is? I'd suggest reducing it to 9, that reduces compilation time by 7 seconds (i.e. optics-core compiles on my machine 16 seconds with tuples up to _19, 9 seconds with tuples up to _9).

I can't imagine someone using tuples so large, surely you define a proper ADT way earlier. If that turns out to be a problem in the future though, we can restore the classes and instances.

phadej commented 5 years ago

Sounds good to me.

adamgundry commented 5 years ago

This sounds fine, though it's a divergence from lens to document.

Relatedly, I wonder whether compilation times are slightly worse with optics compared to lens? I had that impression when porting code, but I haven't tried to measure the difference.

arybczak commented 5 years ago

Relatedly, I wonder whether compilation times are slightly worse with optics compared to lens? I had that impression when porting code, but I haven't tried to measure the difference.

They most likely are, because we add another layer of abstraction (profunctors) over VL representation in most cases, so GHC needs to optimize that away (which works fine as highlighted in tests, but costs compilation time).

domenkozar commented 4 years ago

I just want to raise that I have a tuple of 10 items (possibly even more in the future).

The reason is that https://github.com/nikita-volkov/hasql-th recommends using uncurryN to map items from database into a haskell record.

If anyone hits this, fallback is to use lens that supports 19 items.

arybczak commented 4 years ago

What's the exact use case? If you're getting an N-tuple from a db, shouldn't it be immediately transformed into a record?

domenkozar commented 4 years ago

I want to apply some changes to it first so that it fits the record. In my case I'm getting a string from db in field 5, that needs to be a sum type.

arybczak commented 4 years ago

In theory we can extend this to _19 (but we'd need to split the module so that all these instances can compile in parallel).

But if you use these lenses like that, you'll eventually hit the limit with lens too.

It's quite easy to get to a point of fetching more than 19 columns from a database (in one of the projects I worked on it was over 40).

My experience is that in such case it's better (for future code readers) to explicitly deconstruct the tuple and apply changes in the usual way while constructing the target data type.

arybczak commented 3 years ago

FYI #361 added gposition so you can now get field at any position from data types with a Generic instance (sadly large tuples don't have them, but application code can derive them as needed).