1. 21 Dec, 2012 2 commits
  2. 30 Nov, 2012 1 commit
    • Iustin Pop's avatar
      Remove read instances from our Haskell code · 139c0683
      Iustin Pop authored
      
      
      It turns out that optimising 'read' derived instances (via -O) for
      complex data types (like OpCode, or the various objects) can be slow
      to very slow. Disabling such instances results in (time make
      $all_our_haskell_binaries) large compile-time savings and also smaller
      (unstripped) binaries (by a significant amount):
      
      ghc 6.12:        time  htools sz  hconfd sz
        with read:    4m50s 12,244,694 14,927,928
        no read:      3m30s 10,234,305 12,536,745
      ghc 7.6:
        with read:   14m45s 13,694,761 15,741,755
        no read:      3m40s 11,631,373 13,245,134
      
      So let's remove these instances, since we never use read in production
      for our custom types, and even when debugging in GHCI, we can simply
      use the 'show' representation to create the types, without needing to
      actually parse from strings.
      
      Note: for the very slow ghc 7.6 compilation time, I filled a ticket
      (ghc #7450), since it is surprising(ly slow).
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarMichele Tartara <mtartara@google.com>
      139c0683
  3. 21 Nov, 2012 2 commits
  4. 15 Nov, 2012 1 commit
    • Iustin Pop's avatar
      Cleanup THH function use from built module namespace · 32a569fe
      Iustin Pop authored
      
      
      Currently, THH.hs "injects" into the built code names of library
      functions like Text.JSON.makeObj, Ganeti.JSON.fromObj, etc. built
      directly from strings, via (e.g.)
      
        varE (mkName "makeObj")
      
      This means that the "makeObj" name must exist in the target module,
      i.o.w. must be imported there. This leads to the strange case of
      having to have imports that do not appear at all in the used
      (template) code, but are needed to satisfy this "hidden" dependency;
      look at Ganeti/Jobs.hs before this patch, for example.
      
      This is also not very obvious, because we usually import Text.JSON
      anyway; I only stumbled upon it while doing some cleanup work.
      
      So to clean this up, the current patch changes the THH.hs to use not
      string-derived, but identifier-derived names («'identifier» versus
      «mkName "identifier"»); this is better, as the names must be
      resolvable when compiling THH itself (once), and not when compiling
      the multiple derived modules. As you can see, this allows removal of
      extraneous imports from various modules.
      
      Background information: an `mkName "foo"` results in a name of flavour
      NameS (“An unqualified name; dynamically bound”) or alternatively to a
      qualified name, but still dynamically bound. Whereas what we want is a
      statically bound name: `'foo` results in a NameG flavour, “Global name
      bound outside of the TH AST: An original name”.
      
      One more explanation: the change is similar to going from 'x = eval
      "map"' to 'x = map'; the name is no longer dynamically evaluated, but
      statically when the module is compiled. In our case, previously names
      were bound at target module compile time, now they are bound at THH.hs
      compile time.
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarGuido Trotter <ultrotter@google.com>
      32a569fe
  5. 12 Nov, 2012 1 commit
    • Iustin Pop's avatar
      Remove unsafePerformIO usage · 29a30533
      Iustin Pop authored
      
      
      We need to change a few things, most importantly CLI options defaults,
      but otherwise we already used the path to files from functions which
      were already in the I/O monad, so we don't have to change much of the
      code flow.
      
      Additionally, Path.hs now has an explicit export list, to stop
      accidental leakage of symbols.
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarHelga Velroyen <helgav@google.com>
      29a30533
  6. 16 Oct, 2012 2 commits
  7. 28 Sep, 2012 7 commits
  8. 26 Sep, 2012 7 commits
  9. 05 Sep, 2012 1 commit
    • Iustin Pop's avatar
      Further hlint fixes · 5b11f8db
      Iustin Pop authored
      Commit 2cdaf225, “Re-enable standard hlint warnings”, got it almost
      right. The only problem is that (confusingly) the default set of hints
      is not in HLint.Default, but in HLint.HLint (it includes Default and
      some built-ins).
      
      After changing the lint file to correctly include the defaults, we had
      another 128 suggestions:
      
        - Error: Eta reduce (2)
        - Error: Redundant bracket (4)
        - Error: Redundant do (17)
        - Error: Redundant lambda (7)
        - Error: Redundant return (1)
        - Warning: Avoid lambda (2)
        - Warning: Redundant $ (42)
        - Warning: Redundant bracket (35)
        - Warning: Use : (1)
        - Warning: Use String (4)
        - Warning: Use camelCase (10)
        - Warning: Use section (3)
      
      which are fixed by the current patch. Note that the 10 "Use camelCase"
      were all due to hlint not “knowing” the idiom of ‘case_’ (it does for
      ‘prop_’), for which I filled
      http://code.google.com/p/ndmitchell/issues/detail?id=558
      
      .
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarRené Nussbaumer <rn@google.com>
      5b11f8db
  10. 04 Sep, 2012 1 commit
  11. 28 Aug, 2012 6 commits