Open mikesol opened 4 years ago
Hi @mikesol! This looks interesting! Do you have any metrics on the change, or a snapshot of the difference in generated code? If this were added it would be good to include something like that in the documentation to better understand what this accomplishes.
Below is a benchmark. fuseSetters
results in a roughly 1.8x performance increase on the best run. There is no substantial difference in the generated code.
fuseSetters
makes sense when working with large and/or deeply-nested data structures in a repeated fashion. The basic idea is that a "trunk" setter focuses up to a certain point in the optic, and then the two "branch" setters focus from that point to two different points.
module Main where
import Prelude
import Data.Array (replicate)
import Data.Lens (Setter', _1, _2, over, traversed)
import Data.Tuple (Tuple(..), fst, snd)
import Effect (Effect)
import Effect.Class.Console (log)
fuseSetters :: forall s a b c. Setter' a b -> Setter' a c -> Setter' s a -> Setter' s (Tuple (b -> b) (c -> c))
fuseSetters a b c l = over c (over a fa <<< over b fb)
where
t = l $ Tuple identity identity
fa = fst t
fb = snd t
setter = (traversed <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2)
main :: Effect Unit
main = do
void
( pure
$ over
(fuseSetters (_1 <<< _1) (_2 <<< _2) setter)
(const $ Tuple (const 42) (const 53))
( replicate
1000000
(Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 2 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 2 (Tuple 3 (Tuple (Tuple 0 1) (Tuple 5 8)))))))))))))))))))))
)
)
log "Done"
Best run
07:45 meeshkan-abel@Abel:/tmp/lenz$ time spago run
[info] Installation complete.
[info] Build succeeded.
Done
real 0m3,191s
user 0m8,221s
sys 0m1,889s
module Main where
import Prelude
import Data.Array (replicate)
import Data.Lens (Setter', _1, _2, over, set, traversed)
import Data.Tuple (Tuple(..), fst, snd)
import Effect (Effect)
import Effect.Class.Console (log)
setter = (traversed <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2 <<< _2)
main :: Effect Unit
main = do
void
( pure
$ ((set (setter <<< _1 <<< _1) 42) <<< (set (setter <<< _2 <<< _2) 53))
( replicate
1000000
(Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 2 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 1 (Tuple 2 (Tuple 3 (Tuple (Tuple 0 1) (Tuple 5 8)))))))))))))))))))))
)
)
log "Done"
Best run
07:44 meeshkan-abel@Abel:/tmp/lenz$ time spago run
[info] Installation complete.
[info] Build succeeded.
Done
real 0m6,320s
user 0m14,908s
sys 0m2,051s
This seems like a useful utility to add. I don't have as much knowledge around how this would compose -- do you build up nested fuseSetter
s like fuseSetter a (fuseSetter b c identity) d
? -- and I'd appreciate hearing what @LiamGoodacre or @MonoidMusician think before confirming for sure that I'd like to add it to the library. If either of them think it's good as-is then I welcome a PR!
Thanks for the questions and feedback!
Composition looks like this:
module Main where
import Prelude
import Data.Array (replicate)
import Data.Lens (Setter', _1, _2, over, set, traversed)
import Data.Tuple (Tuple(..), fst, snd)
import Effect (Effect)
import Effect.Class.Console (log)
fuseSetters :: forall s a b c. Setter' a b -> Setter' a c -> Setter' s a -> Setter' s (Tuple (b -> b) (c -> c))
fuseSetters a b c l = over c (over a fa <<< over b fb)
where
t = l $ Tuple identity identity
fa = fst t
fb = snd t
cTuple :: forall a b c. a -> b -> c -> Tuple a b
cTuple a b = const $ Tuple a b
main :: Effect Unit
main = do
log
( show
$ over (fuseSetters _1 (fuseSetters _1 _2 _2) _2)
( cTuple
((+) 55)
(cTuple (flip (-) 101) ((*) 57))
)
(Tuple 0 (Tuple 0 (Tuple 1 2)))
)
yields
06:35 meeshkan-abel@Abel:/tmp/lenz$ spago run
[info] Installation complete.
[info] Build succeeded.
(Tuple 0 (Tuple 55 (Tuple -100 114)))
So the input type is a tree resembling the tree of the fused setters.
fuseSetters :: forall s a b c. Setter' a b -> Setter' a c -> Setter' s a -> Setter' s (Tuple (b -> b) (c -> c))
fuseSetters a b c l = over c (over a fa <<< over b fb) ...
The over c
part seems a little redundant?
fuseSettersV2 :: forall a b c. Setter' a b -> Setter' a c -> Setter' a (Tuple (b -> b) (c -> c))
fuseSettersV2 ba ca l = (over ba fa <<< over ca fb) ...
where
fuseSetters b2a c2a a2s
-- is
a2s <<< fuseSettersV2 b2a c2a
Nice find! It makes the syntax cleaner too. I've updated the PR.
module Main where
import Prelude
import Data.Array (replicate)
import Data.Lens (Setter', Setter, _1, _2, over, set, traversed)
import Data.Tuple (Tuple(..), fst, snd)
import Effect (Effect)
import Effect.Class.Console (log)
fuseSetters :: forall a b c. Setter' a b -> Setter' a c -> Setter' a (Tuple (b -> b) (c -> c))
fuseSetters ba ca l = (over ba fa <<< over ca fb)
where
t = l (Tuple identity identity)
fa = fst t
fb = snd t
cTuple :: forall a b c. a -> b -> c -> Tuple a b
cTuple a b _ = Tuple a b
main :: Effect Unit
main = do
log
( show
$ over
(_2 <<< (fuseSetters _1 (_2 <<< (fuseSetters _1 _2))))
( cTuple
((+) 55)
(cTuple (flip (-) 101) ((*) 57))
)
(Tuple 0 (Tuple 0 (Tuple 1 2)))
)
I'm not sure what value this provides. Isn't the alternative to setter <<< fuseSetters (_1 <<< _1) (_2 <<< _2)
not (set (setter <<< _1 <<< _1) 42) <<< (set (setter <<< _2 <<< _2) 53)
but over setter (set (_1 <<< _1) 42 <<< set (_2 <<< _2) 53)
? (Sorry I didn't run that through a typechecker.) If the same idea and performance can be accomplished within the existing idiom, it seems cleaner to me to do so, especially to avoid dealing with const
/cTuple
. Unless there's some value added by using Setter
s here that I'm missing …
After looking over it again, is it just the ability to separate the data from the setter while maintaining performance? I wonder if there's a way to achieve that without using Setter' a (Tuple (b -> b) (c -> c))
and so many const
s, because I would think that having to rewrite the data to use const
s would make it less convenient to use, but idk … Would Setter' a (Tuple b c)
work as the return type?
Actually, after some more research, I think that Setter' a b -> Setter' a c -> Setter' a (Tuple b c)
(and probably fuseSetters
already) runs into the issue that it's not lawful when the lenses/setters overlap, c.f. https://stackoverflow.com/a/23322201.
This is a bit over my head, but what I would say is that if the idiom you've implemented works, it's much better to use that than fuseSetters
. At the end of the day, we use fuseSetters
to speed up performance, so anything that achieves a similar effect in a more elegant way is much better. The only issue is that we have it in production code, so it may be a while before we can test it out thoroughly.
Another reason not to include it would be that it's too use-case specific. Most folks won't need speed optimization on setters.
Hello!
We've been using a function called
fuseSetters
a fair bit in production to speed up code where several setters are called on complex records (in our case, adding custom OpenAPI tags to a large OpenAPI spec).Here is the function with the output. If folks find it useful, I can make a PR to include it in the library:
yields
Thanks and let me know!