- rGHC Glasgow Haskell Compiler
No Unit Test Coverage
- Build Status
Buildable 25592 Build 65103: [GHC] Linux/amd64: Continuous Integration Build 65102: [GHC] OSX/amd64: Continuous Integration Build 65101: [GHC] Windows/amd64: Continuous Integration Build 65100: arc lint + arc unit
Good news, it seems the current set of changes, when make fasttest -j8 is run, produce (on my machine, of course):
Unexpected stat failures: perf/compiler/T5631.run T5631 [stat too good] (normal) perf/haddock/haddock.base.run haddock.base [stat too good] (normal) perf/haddock/haddock.Cabal.run haddock.Cabal [stat too good] (normal) perf/should_run/T9203.run T9203 [stat too good] (normal)
There are several steps to go through, still, such as:
- running tests for every commit since one individual change might introduce a regression that the other commits overcome with increased performance
- Use make slowtest instead of make fasttest since running all tests might reveal more information about whether this change is positive or not.
Expected T9203(normal) bytes allocated: 98360576 +/-5% Lower bound T9203(normal) bytes allocated: 93442547 Upper bound T9203(normal) bytes allocated: 103278605 Actual T9203(normal) bytes allocated: 46371896 Deviation T9203(normal) bytes allocated: -52.9 %
Expected T5631(normal) bytes allocated: 1161885448 +/-5% Lower bound T5631(normal) bytes allocated: 1103791175 Upper bound T5631(normal) bytes allocated: 1219979721 Actual T5631(normal) bytes allocated: 1055392736 Deviation T5631(normal) bytes allocated: -9.2 %
Expected haddock.base(normal) bytes allocated: 25913205656 +/-5% Lower bound haddock.base(normal) bytes allocated: 24617545373 Upper bound haddock.base(normal) bytes allocated: 27208865939 Actual haddock.base(normal) bytes allocated: 24007020952 Deviation haddock.base(normal) bytes allocated: -7.4 %
Expected haddock.Cabal(normal) bytes allocated: 27520214496 +/-5% Lower bound haddock.Cabal(normal) bytes allocated: 26144203771 Upper bound haddock.Cabal(normal) bytes allocated: 28896225221 Actual haddock.Cabal(normal) bytes allocated: 25275864688 Deviation haddock.Cabal(normal) bytes allocated: -8.2 %
There are a number of changes in this patch, but none of them are commented in any way. So maybe someone will change them all back in a while! It would really help to document the intent, rather than just change the code.
Sometimes a single Note, which can be pointed to from several places, is the easiest way to explain how all the moving parts fit together.
I've made a few notes to that effects in the individual places.
Why this change? Does it fuse better? Why?
Why? How does inlining help?
(Incidentally, since it's recursive the pragma will do nothing.)
Why? Please explain.
Hm, it seems I spoke too soon. I am not sure of the reason, but it seems that the tests that output stat too good do so in the latest master branch as well, and with identical results. Can anyone confirm this? I had expected that all performance tests in the latest master would pass without being too bad or too good, at least when ran with make test, but it seems I was wrong, both on that and about these changes being strictly positive. I cannot claim they are when the same improvement appears without them.
I'll continue trying things out.
- Allow fusion for unzip4
- Allow fusion for unzip5
- Allow fusion for unzip6
- Allow fusion for unzip7
- Add comment explaining INLINE for unzips
- Add inline rule to mapAndUnzip3M
- Add inline rule to mapAndUnzip4M
- Add inline rule to mapAndUnzip5M
- Add explanation to mapAndUnzipM changes
- Add inline for zipWith3M
- Add inline for zipWith3M_
- Add inline for zipWith4M
- Add comment explaining zipWithM inline
This differential has been updated to reflect the changes from D5241 (some of the previous changes here have already been implemented by that diff), along with some comments explaining the changes.
@bgamari this is a low priority issue, but could you take a look when possible?
@simonpj the changes I've now introduced should address your requests for comment. I apologize for taking so long to update a simple issue.
I don't like having comments like this! We should fix the original problem.
The inline principle for @zipWith3M@ and @zipWith3M_@ is the same as for zipWithM' and 'zipWithM_' in "Control.Monad". No comment or explanation is given in that module as to why 'zipWith' has an @INLINE@ pragma, but it stands to reason that if it was added in the past and not removed or addressed in any way, it is overall useful (but still undocumented, which would be nice to address).
I'll push a patch and you can then change this one to refer to the new Notes in my patch
@simonpj sorry, but what do you specifically mean by this:
I don't having like comments like this
There could be many things wrong with the comments: they're too long, they're not concise enough, they're too numerous and should therefore be condensed into fewer comments, or just one in an easily accessible location, or all of the previous things together.
Sorry not to be clear! I meant that I don't like a comment that says "I don't know why this code is right, but it's the same as that code over there, which also lacks any comment". Clearly the right thing is to add the missing comment which I have done in
@simonpj you're right - I forgot to raise that point in this discussion so that someone could add those comments. I didn't do it myself since I'm not that familiar with the codebase yet.
Thank you that MR. As soon as it's merged and lands on this instance's master, I'll fix this Diff and push the changes.
Lovely. Some minor suggestions.
Is this Note referred to? I think not. Perhaps it'd make sense to refer to this note from functions defined in this module; and this Note can refer on to the Note in List.hs?
Can you refer directly to the Note?
Same comment here
Is there a Note in GHC..List to refer to? If so, do; if not, fine.
I've just sent you an email.
I apologise for this whole situation, although I'd like to point out that this happened because of circumstances beyond my control, and also that it has happened before - I could not write to email@example.com a few months ago, so I asked a colleague to write for me. It seems to have been a problem with Outlook blacklisting some types of messages.