RULES in binary

newer
TypeLits and type families wrt....

older
Handling overflow and division by...

Simon Peyton Jones

24 Jul 2015 24 Jul '15

7:44 a.m.

Lennart In the binary library I'm seeing lots of these warnings: libraries/binary/src/Data/Binary/Get.hs:420:1: warning: Rule "getWord16le/readN" may never fire because 'getWord16le' might inline first Probable fix: add an INLINE[n] or NOINLINE[n] pragma on this function libraries/binary/src/Data/Binary/Builder/Base.hs:510:1: warning: Rule "flush/flush" may never fire because 'flush' might inline first Probable fix: add an INLINE[n] or NOINLINE[n] pragma on this function The warnings look right to me: currently everything is very fragile and may not work as you intend. You may want to look into this? Simon

Attachments:

attachment.html (text/html — 3.6 KB)

Show replies by date

Lennart Kolmodin

26 Jul 26 Jul

4:50 p.m.

Yes, this has been on my todo for a long time :) Essentially all inlinings/rules in binary should be gone through and confirmed whether they're still needed. I had a look now to get some insight. Since a few versions GHC warns in this way when something might not go the way it was intended, a great way to learn more about how inlining and rules work and to avoid surprises. All these warnings are proof of my poor understanding when I implemented it. Naturally it should all be fixed. Here's how I reasoned when implementing it; In Data.Binary.Get we have functions we always want to inline, even if GHC doesn't think it's a good idea. Therefore there are both INLINE pragmas as well as RULES to achieve this. GHC now warns that the function might get inlined before the rule triggers, which is ok since they do the same thing. We should probably re-evaluate whether always inlining still is a good idea. If it is, we can keep the RULES to inline, and change the INLINE to NOINLINE and let the RULES do their job. In Data.Binary.Internal.Get we attempt a trick where applicative code can become more efficient. It tries to rewrite the components of an expression "f <*> g <*> h" into something that does f, g and h with a single bounds check (the check for "do we have enough input bytes to continue?"). This trick relies so much on that the user's code has been inlined properly that it probably very rarely fires in a real application. It does wonders in the unrealistic micro benchmark, though :) Probably those rules can be removed without any real code suffering. I'd like to add some more real world benchmarks, and finally test with the changes proposed above. Lennart 2015-07-24 13:44 GMT+02:00 Simon Peyton Jones :

...

Lennart

In the binary library I’m seeing lots of these warnings:

libraries/binary/src/Data/Binary/Get.hs:420:1: warning:

Rule "getWord16le/readN" may never fire

because ‘getWord16le’ might inline first

Probable fix: add an INLINE[n] or NOINLINE[n] pragma on this function

libraries/binary/src/Data/Binary/Builder/Base.hs:510:1: warning:

Rule "flush/flush" may never fire

because ‘flush’ might inline first

Probable fix: add an INLINE[n] or NOINLINE[n] pragma on this function

The warnings look right to me: currently everything is very fragile and may not work as you intend.

You may want to look into this?

Simon

Joachim Breitner

5:06 p.m.

Hi, Am Sonntag, den 26.07.2015, 22:50 +0200 schrieb Lennart Kolmodin:

...

This trick relies so much on that the user's code has been inlined properly that it probably very rarely fires in a real application. It does wonders in the unrealistic micro benchmark, though :)

what is the name of the rule? Can you find it on this list: https://gist.github.com/nomeata/071c1f87450cf668bbeb Greetings, 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

Lennart Kolmodin

27 Jul 27 Jul

5:47 a.m.

2015-07-26 23:06 GMT+02:00 Joachim Breitner :

...

Hi,

Am Sonntag, den 26.07.2015, 22:50 +0200 schrieb Lennart Kolmodin:

...
This trick relies so much on that the user's code has been inlined properly that it probably very rarely fires in a real application. It does wonders in the unrealistic micro benchmark, though :)

what is the name of the rule? Can you find it on this list: https://gist.github.com/nomeata/071c1f87450cf668bbeb

The rules; {-# RULES "<$> to <*>" forall f g. (<$>) f g = returnG f <*> g "readN/readN merge" forall n m f g. apG (readN n f) (readN m g) = readN (n+m) (\bs -> f bs $ g (B.unsafeDrop n bs)) "returnG/readN swap" [~1] forall f. returnG f = readN 0 (const f) "readN 0/returnG swapback" [1] forall f. readN 0 f = returnG (f B.empty) #-}

...

From your list (all of stackage with -ddump-rule-firings? brilliant! :)); 399 Rule fired: <$> to <*> 106 Rule fired: readN/readN merge 1373 Rule fired: returnG/readN swap 2351 Rule fired: readN 0/returnG swapback

The "readN/readN merge" rule is what gives the speedup, the other rules are needed just to get the code lined up for the merge rule to fire. So it fired 106 times. That's not a lot. When compiling one of the binary benchmarks it fires 33 times. It might fire less often than it could, because the "<$> to <*>" rule might not get to fire as often as it wants (just like the warning says). But, I can't slap a INLINE or NOINLINE on <$> though, since it's not part of the binary library. So even if it does fire for some program out there, I'm still inclined to remove the rules. It's a little bit too much black magic. Lennart

...

Greetings, 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

Simon Peyton Jones

4:02 a.m.

Terrific. If a RULE and an inlining “do the same thing”, the RULE is usually to be preferred because it duplicates less code. Simon From: Lennart Kolmodin [mailto:kolmodin@gmail.com] Sent: 26 July 2015 21:50 To: Simon Peyton Jones Cc: ghc-devs@haskell.org Subject: Re: RULES in binary Yes, this has been on my todo for a long time :) Essentially all inlinings/rules in binary should be gone through and confirmed whether they're still needed. I had a look now to get some insight. Since a few versions GHC warns in this way when something might not go the way it was intended, a great way to learn more about how inlining and rules work and to avoid surprises. All these warnings are proof of my poor understanding when I implemented it. Naturally it should all be fixed. Here's how I reasoned when implementing it; In Data.Binary.Get we have functions we always want to inline, even if GHC doesn't think it's a good idea. Therefore there are both INLINE pragmas as well as RULES to achieve this. GHC now warns that the function might get inlined before the rule triggers, which is ok since they do the same thing. We should probably re-evaluate whether always inlining still is a good idea. If it is, we can keep the RULES to inline, and change the INLINE to NOINLINE and let the RULES do their job. In Data.Binary.Internal.Get we attempt a trick where applicative code can become more efficient. It tries to rewrite the components of an expression "f <*> g <*> h" into something that does f, g and h with a single bounds check (the check for "do we have enough input bytes to continue?"). This trick relies so much on that the user's code has been inlined properly that it probably very rarely fires in a real application. It does wonders in the unrealistic micro benchmark, though :) Probably those rules can be removed without any real code suffering. I'd like to add some more real world benchmarks, and finally test with the changes proposed above. Lennart 2015-07-24 13:44 GMT+02:00 Simon Peyton Jones mailto:simonpj@microsoft.com>: Lennart In the binary library I’m seeing lots of these warnings: libraries/binary/src/Data/Binary/Get.hs:420:1: warning: Rule "getWord16le/readN" may never fire because ‘getWord16le’ might inline first Probable fix: add an INLINE[n] or NOINLINE[n] pragma on this function libraries/binary/src/Data/Binary/Builder/Base.hs:510:1: warning: Rule "flush/flush" may never fire because ‘flush’ might inline first Probable fix: add an INLINE[n] or NOINLINE[n] pragma on this function The warnings look right to me: currently everything is very fragile and may not work as you intend. You may want to look into this? Simon

Lennart Kolmodin

6:02 a.m.

2015-07-27 10:02 GMT+02:00 Simon Peyton Jones :

...

Terrific.

If a RULE and an inlining “do the same thing”, the RULE is usually to be preferred because it duplicates less code.

As I've understood it, I'll still need an (NO)INLINE pragma. GHC will warn that the RULE might not fire since the function might get inlined, and GHC might inline without me explicitly annotating with the INLINE pragma. I could change the INLINE to NOINLINE and always let the RULE do the job. Right?

...

Simon

*From:* Lennart Kolmodin [mailto:kolmodin@gmail.com] *Sent:* 26 July 2015 21:50 *To:* Simon Peyton Jones *Cc:* ghc-devs@haskell.org *Subject:* Re: RULES in binary

Yes, this has been on my todo for a long time :)

Essentially all inlinings/rules in binary should be gone through and confirmed whether they're still needed.

I had a look now to get some insight.

Since a few versions GHC warns in this way when something might not go the way it was intended, a great way to learn more about how inlining and rules work and to avoid surprises.

All these warnings are proof of my poor understanding when I implemented it. Naturally it should all be fixed.

Here's how I reasoned when implementing it;

In Data.Binary.Get we have functions we always want to inline, even if GHC doesn't think it's a good idea. Therefore there are both INLINE pragmas as well as RULES to achieve this. GHC now warns that the function might get inlined before the rule triggers, which is ok since they do the same thing.

We should probably re-evaluate whether always inlining still is a good idea. If it is, we can keep the RULES to inline, and change the INLINE to NOINLINE and let the RULES do their job.

In Data.Binary.Internal.Get we attempt a trick where applicative code can become more efficient. It tries to rewrite the components of an expression "f <*> g <*> h" into something that does f, g and h with a single bounds check (the check for "do we have enough input bytes to continue?").

This trick relies so much on that the user's code has been inlined properly that it probably very rarely fires in a real application. It does wonders in the unrealistic micro benchmark, though :)

Probably those rules can be removed without any real code suffering. I'd like to add some more real world benchmarks, and finally test with the changes proposed above.

Lennart

2015-07-24 13:44 GMT+02:00 Simon Peyton Jones :

Lennart

In the binary library I’m seeing lots of these warnings:

libraries/binary/src/Data/Binary/Get.hs:420:1: warning:

Rule "getWord16le/readN" may never fire

because ‘getWord16le’ might inline first

Probable fix: add an INLINE[n] or NOINLINE[n] pragma on this function

libraries/binary/src/Data/Binary/Builder/Base.hs:510:1: warning:

Rule "flush/flush" may never fire

because ‘flush’ might inline first

Probable fix: add an INLINE[n] or NOINLINE[n] pragma on this function

The warnings look right to me: currently everything is very fragile and may not work as you intend.

You may want to look into this?

Simon

Simon Peyton Jones

6:43 a.m.

Correct. Or if you want the inlning, you can delay it by saying INLINE [n], or indeed NOINLINE [n], which will ensure that the inlining does not happen until phase n S From: Lennart Kolmodin [mailto:kolmodin@gmail.com] Sent: 27 July 2015 11:02 To: Simon Peyton Jones Cc: ghc-devs@haskell.org Subject: Re: RULES in binary 2015-07-27 10:02 GMT+02:00 Simon Peyton Jones mailto:simonpj@microsoft.com>: Terrific. If a RULE and an inlining “do the same thing”, the RULE is usually to be preferred because it duplicates less code. As I've understood it, I'll still need an (NO)INLINE pragma. GHC will warn that the RULE might not fire since the function might get inlined, and GHC might inline without me explicitly annotating with the INLINE pragma. I could change the INLINE to NOINLINE and always let the RULE do the job. Right? Simon From: Lennart Kolmodin [mailto:kolmodin@gmail.commailto:kolmodin@gmail.com] Sent: 26 July 2015 21:50 To: Simon Peyton Jones Cc: ghc-devs@haskell.orgmailto:ghc-devs@haskell.org Subject: Re: RULES in binary Yes, this has been on my todo for a long time :) Essentially all inlinings/rules in binary should be gone through and confirmed whether they're still needed. I had a look now to get some insight. Since a few versions GHC warns in this way when something might not go the way it was intended, a great way to learn more about how inlining and rules work and to avoid surprises. All these warnings are proof of my poor understanding when I implemented it. Naturally it should all be fixed. Here's how I reasoned when implementing it; In Data.Binary.Get we have functions we always want to inline, even if GHC doesn't think it's a good idea. Therefore there are both INLINE pragmas as well as RULES to achieve this. GHC now warns that the function might get inlined before the rule triggers, which is ok since they do the same thing. We should probably re-evaluate whether always inlining still is a good idea. If it is, we can keep the RULES to inline, and change the INLINE to NOINLINE and let the RULES do their job. In Data.Binary.Internal.Get we attempt a trick where applicative code can become more efficient. It tries to rewrite the components of an expression "f <*> g <*> h" into something that does f, g and h with a single bounds check (the check for "do we have enough input bytes to continue?"). This trick relies so much on that the user's code has been inlined properly that it probably very rarely fires in a real application. It does wonders in the unrealistic micro benchmark, though :) Probably those rules can be removed without any real code suffering. I'd like to add some more real world benchmarks, and finally test with the changes proposed above. Lennart 2015-07-24 13:44 GMT+02:00 Simon Peyton Jones mailto:simonpj@microsoft.com>: Lennart In the binary library I’m seeing lots of these warnings: libraries/binary/src/Data/Binary/Get.hs:420:1: warning: Rule "getWord16le/readN" may never fire because ‘getWord16le’ might inline first Probable fix: add an INLINE[n] or NOINLINE[n] pragma on this function libraries/binary/src/Data/Binary/Builder/Base.hs:510:1: warning: Rule "flush/flush" may never fire because ‘flush’ might inline first Probable fix: add an INLINE[n] or NOINLINE[n] pragma on this function The warnings look right to me: currently everything is very fragile and may not work as you intend. You may want to look into this? Simon

3585

Age (days ago)

3588

Last active (days ago)

List overview

Download

6 comments

3 participants

participants (3)

Joachim Breitner
Lennart Kolmodin
Simon Peyton Jones

RULES in binary

Simon Peyton Jones

Lennart Kolmodin

Joachim Breitner

Lennart Kolmodin

Simon Peyton Jones

Lennart Kolmodin

Simon Peyton Jones

tags

participants (3)