Opened 14 months ago

Closed 9 months ago

Last modified 9 months ago

#8704 closed feature request (fixed)

Use GHC.Exts.build 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 Revisions:

Description

randoms, randomRs could take advantage of list fusion.

A commit is attached for consideration.

Attachments (1)

0001-Use-GHC.Exts.build-in-randoms-randomRs-to-achieve-fu.patch (2.4 KB) - added by ion1 14 months ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 14 months ago by jstolarek

  • Status changed from new to patch

comment:2 Changed 14 months 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 14 months 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 14 months ago by ion1 (previous) (diff)

comment:4 Changed 14 months 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 14 months ago by rrnewton

  • Owner set to rrnewton

comment:6 Changed 9 months ago by thomie

  • Cc rrnewton added
  • Milestone set to 7.8.4
  • Resolution set to fixed
  • Status changed from patch to closed
  • Version changed from 7.6.3 to 7.9

This has been committed in 4695ffa366f659940369f05e419a4f2249c3a776.

comment:7 Changed 9 months ago by thoughtpolice

  • Milestone changed from 7.8.4 to 7.10.1
Note: See TracTickets for help on using tickets.