Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#8704 closed feature request (fixed)

Use in randoms, randomRs to achieve fusion

Reported by: ion1 Owned by: rrnewton
Priority: normal Milestone: 7.10.1
Component: libraries/random Version: 7.9
Keywords: fusion Cc: rrnewton
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: #4218 Differential Rev(s):
Wiki Page:


randoms, randomRs could take advantage of list fusion.

A commit is attached for consideration.

Attachments (1) (2.4 KB) - added by ion1 6 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 6 years ago by jstolarek

Status: newpatch

comment:2 Changed 6 years ago by nomeata

Thanks for the patch, ion1.

Just checking: Did you make sure that the fusion works as desired, e.g. by writing a very small example and comparing the resulting Core before and after?

comment:3 Changed 6 years ago by ion1

Yes, I tested with

main = do { gen <- newStdGen; mapM_ print (randoms gen :: [Int]) }

which resulted in the list data structure being optimized away entirely (along with the pattern match for the empty case in mapM_).

I got around to doing some benchmarking after posting the patch and I seem unable to come up with code for which randoms fusion makes a difference in performance, though. Unless someone else thinks of something I didn’t, this might not be worth it.

The seq fix for part of #4218 can be done independently.

Last edited 6 years ago by ion1 (previous) (diff)

comment:4 Changed 6 years ago by nomeata

Thanks. From my POV it is worth adding even if you can’t measure a performance gain; it is still good to know that nice code is being generated. But of course it is up to Ryan Newton (random maintainer) to decide this.

comment:5 Changed 6 years ago by rrnewton

Owner: set to rrnewton

comment:6 Changed 5 years ago by thomie

Cc: rrnewton added
Milestone: 7.8.4
Resolution: fixed
Status: patchclosed

This has been committed in 4695ffa366f659940369f05e419a4f2249c3a776.

comment:7 Changed 5 years ago by thoughtpolice

Note: See TracTickets for help on using tickets.