Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add template haskell helpers to generate Unbox instances #2420

Merged
merged 8 commits into from
Aug 3, 2023

Conversation

adithyaov
Copy link
Member

No description provided.

@rnjtranjan rnjtranjan self-assigned this Jun 28, 2023
Copy link
Member

@harendra-kumar harendra-kumar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid non-boot dependencies and move this to streamly-core package?

@adithyaov adithyaov force-pushed the th-stuff branch 3 times, most recently from 5ab8c2d to 603dea2 Compare July 5, 2023 18:01
@adithyaov
Copy link
Member Author

Check perf of Generic vs Template Haskell

@adithyaov
Copy link
Member Author

adithyaov commented Jul 10, 2023

data CustomDT1 a b
    = CDT1C1
    | CDT1C2 a
    | CDT1C3 a b
    deriving (Generic, Show)
type CustomDT1_ = CustomDT1 Int Bool

instance (Unbox a, Unbox b) => Unbox (CustomDT1 a b)

data CustomDT2 a b
    = CDT2C1
    | CDT2C2 a
    | CDT2C3 a b
    deriving (Show)
type CustomDT2_ = CustomDT2 Int Bool

$(makeUnbox ''CustomDT2)
All
  serializeDeserializeTimes CDT1C1 (Generic): OK (0.22s)
    208  μs ±  11 μs
  serializeDeserializeTimes CDT2C1 (TH):      OK (0.18s)
    92.7 μs ± 6.3 μs
  serializeDeserializeTimes CDT1C2 (Generic): OK (0.19s)
    370  μs ±  25 μs
  serializeDeserializeTimes CDT2C2 (TH):      OK (0.13s)
    62.0 μs ± 5.8 μs
  serializeDeserializeTimes CDT1C3 (Generic): OK (0.30s)
    583  μs ±  37 μs
  serializeDeserializeTimes CDT2C3 (TH):      OK (0.18s)
    86.7 μs ± 5.7 μs

Template haskell version looks significantly faster.

test/Streamly/Test/Data/Unbox.hs Outdated Show resolved Hide resolved
core/src/Streamly/Internal/Data/Unbox/TH.hs Outdated Show resolved Hide resolved
core/src/Streamly/Internal/Data/Unbox/TH.hs Outdated Show resolved Hide resolved
("Attempting to get size with no constructors (" ++
$(lift (pprint headTy)) ++ ")")|]
-- One constructor with no fields is a unit type. Size of a unit type is
-- 1.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was some special handling for unit in generic deriving. May want to look more closely in the generic deriving code to make sure nothing is missed..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests seem to pass

core/src/Streamly/Internal/Data/Unbox/TH.hs Show resolved Hide resolved
test/Streamly/Test/Data/Unbox.hs Outdated Show resolved Hide resolved
@adithyaov
Copy link
Member Author

Compare store vs streamly serialization

@adithyaov adithyaov force-pushed the th-stuff branch 3 times, most recently from fb1ab65 to 9102fba Compare July 29, 2023 07:50
@adithyaov adithyaov changed the base branch from master to split-unbox-tests July 29, 2023 08:03
Copy link
Member

@harendra-kumar harendra-kumar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should expose deriveUnbox and deriveUnboxWith via the Internal.Data.Unbox module as well.

Combine Unbox and Serialize exposed modules.

benchmark/Streamly/Benchmark/Data/Unbox.hs Outdated Show resolved Hide resolved
core/src/Streamly/Internal/Data/Unbox.hs Show resolved Hide resolved
core/src/Streamly/Internal/Data/Unbox/TH.hs Outdated Show resolved Hide resolved
@adithyaov
Copy link
Member Author

See #2489

Copy link
Member

@harendra-kumar harendra-kumar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the CIs.

@adithyaov adithyaov merged commit ee9a47e into split-unbox-tests Aug 3, 2023
12 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants