Trying to fix an efficiency issue noted in a TODO in SAT.hs
compiler/simplCore/SAT.hs has a TODO comment about the fact that it does a fair bit of appending onto the ends of lists, and that should be done differently. I made an attempt to fix it. The complexity of the recursion, however, leaves me uncertain as to whether I really did or not. I've attached a diff and I hope someone will be able to take a look at it. The only use of Sequence.fromList is source line 172, and the only significant use of Foldable.toList (aside from pretty-printing) is on source line 402. Note that the use of Sequence may be temporary—I want to get the right code structure down before choosing the best data structure. Thanks, David Feuer
Hi David, Am Samstag, den 06.09.2014, 23:05 -0400 schrieb David Feuer:
compiler/simplCore/SAT.hs has a TODO comment about the fact that it does a fair bit of appending onto the ends of lists, and that should be done differently. I made an attempt to fix it.
Great! We need people to work on GHC’s own performance. Did you profile first, and did it show up there? You know, premature optimization... so it might be that your fix is a nice improvement and useful exercise (and very welcome as such), but without much real-world effect. Nofib can report compiler runtimes. Another way to measuring the effect is compare the output of the perf/compiler test cases with "make VERBOSE=4". BTW, code review is easier on Phabricator. Maye you want to get started using that? See https://ghc.haskell.org/trac/ghc/wiki/Phabricator for instructions. Greeting, Joachim -- Joachim “nomeata” Breitner mail@joachim-breitner.de • http://www.joachim-breitner.de/ Jabber: nomeata@joachim-breitner.de • GPG-Key: 0xF0FBF51F Debian Developer: nomeata@debian.org
The static argument transformation is currently run only when –fstatic-argument-transformation is specified; i.e almost never. It has not received any love for some time. I note the following comment from Max Bolingbroke in DynFlags.hs -- , ([2], Opt_StaticArgumentTransformation) -- Max writes: I think it's probably best not to enable SAT with -O2 for the -- 6.10 release. The version of SAT in HEAD at the moment doesn't incorporate -- several improvements to the heuristics, and I'm concerned that without -- those changes SAT will interfere with some attempts to write "high -- performance Haskell", as we saw in some posts on Haskell-Cafe earlier -- this year. In particular, the version in HEAD lacks the tail call -- criterion, so many things that look like reasonable loops will be -- turned into functions with extra (unnecessary) thunk creation. I’m therefore reluctant to commit refactoring to SAT. They will, in effect, be entirely un-tested. There is a ticket here: https://ghc.haskell.org/trac/ghc/ticket/9374 Do by all means add your diffs to it, so that if anyone (possibly even you) picks it up they will get the benefit of your work. Simon From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of David Feuer Sent: 07 September 2014 04:06 To: ghc-devs Subject: Trying to fix an efficiency issue noted in a TODO in SAT.hs compiler/simplCore/SAT.hs has a TODO comment about the fact that it does a fair bit of appending onto the ends of lists, and that should be done differently. I made an attempt to fix it. The complexity of the recursion, however, leaves me uncertain as to whether I really did or not. I've attached a diff and I hope someone will be able to take a look at it. The only use of Sequence.fromList is source line 172, and the only significant use of Foldable.toList (aside from pretty-printing) is on source line 402. Note that the use of Sequence may be temporary—I want to get the right code structure down before choosing the best data structure. Thanks, David Feuer
participants (3)
-
David Feuer -
Joachim Breitner -
Simon Peyton Jones