Opened 7 years ago

Last modified 8 months ago

#3379 new task

GHC should use the standard binary package

Reported by: igloo Owned by:
Priority: normal Milestone:
Component: Compiler Version: 6.10.4
Keywords: Cc: dterei, kolmodin
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

GHC should use the standard binary package, rather than reimplementing its functionality itself. If the current binary package is slower than GHC's Binary, then we should fix that.

There's some discussion about this in #3041.

Change History (13)

comment:1 Changed 7 years ago by batterseapower

Annotations should also use the standard binary package for serialisation if it becomes a boot library.

comment:2 Changed 6 years ago by igloo

  • Milestone changed from 7.0.1 to 7.0.2

comment:3 Changed 6 years ago by igloo

  • Milestone changed from 7.0.2 to 7.2.1

comment:4 Changed 5 years ago by igloo

  • Milestone changed from 7.2.1 to 7.6.1
  • Type of failure set to None/Unknown
[16:40] < JaffaCake> Igloo: I don't know if we can use binary as it stands, we
                     have a customised Binary class that passes some state
                     around
[16:40] < copumpkin> there really needs to be a Get/Put transformer
[16:41] < JaffaCake> yep
[16:41] < JaffaCake> but also we'd have to be careful that the layers got
                     optimised away properly

comment:5 Changed 5 years ago by dterei

  • Cc dterei added

comment:6 Changed 4 years ago by igloo

  • Milestone changed from 7.6.1 to 7.6.2

comment:7 Changed 2 years ago by thoughtpolice

  • Milestone changed from 7.6.2 to 7.10.1

Moving to 7.10.1.

comment:8 Changed 2 years ago by thomie

GHC still has is its own Binary module, see compiler/utils/Binary.hs and:

$ git grep 'import Binary' compiler/

comment:9 Changed 22 months ago by thoughtpolice

  • Milestone changed from 7.10.1 to 7.12.1

Moving to 7.12.1 milestone; if you feel this is an error and should be addressed sooner, please move it back to the 7.10.1 milestone.

comment:10 Changed 13 months ago by thomie

  • Cc kolmodin jstolarek added

The binary package received a portable implementation of instance Binary Integer in the following commit: https://github.com/kolmodin/binary/commit/2109b28f4d3919dbedcf42c97e51471165278a5c

GHC's version looks like this:

instance Binary Integer where
     -- XXX This is hideous
     put_ bh i = put_ bh (show i)
     get bh = do str <- get bh
                 case reads str of
                     [(i, "")] -> return i
                     _ -> fail ("Binary Integer: got " ++ show str)

As mentioned in Phab:D1165, compiler/utils/Binary.hs contains also a commented out implementation of instance Binary Integer. It is supposedly more efficient than the one in the binary package (but not portable?).

This should all be cleared and cleaned up.

comment:11 Changed 13 months ago by thoughtpolice

  • Milestone changed from 7.12.1 to 8.0.1

Milestone renamed

comment:12 Changed 13 months ago by jstolarek

  • Cc jstolarek removed

comment:13 Changed 8 months ago by thomie

  • Milestone 8.0.1 deleted
Note: See TracTickets for help on using tickets.