tdammers (Tobias Dammers)Administrator
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 16 2017, 8:24 AM (38 w, 6 d)
Roles
Administrator

Recent Activity

Mon, Jul 9

tdammers added a comment to D4931: Fixes an issue with deprecated symbols that are explicitly imported not throwing a warning.

I've looked a bit deeper, and it turns out that the "bug" isn't really a bug - essentially what bytestring is doing here is re-exporting findSubstring from Data.ByteString from Data.ByteString.Char8. More details in the ticket (Trac #15337).

Mon, Jul 9, 9:31 AM
tdammers accepted D4924: Add a strict version of foldMap to Foldable.
Mon, Jul 9, 7:14 AM
tdammers accepted D4945: SAT: Fix hacky name shadowing.
Mon, Jul 9, 6:35 AM
tdammers added a comment to D4931: Fixes an issue with deprecated symbols that are explicitly imported not throwing a warning.

Well, here's the relevant bit from the build log:

Mon, Jul 9, 6:30 AM

Thu, Jul 5

tdammers added a comment to D4915: split-obj: disable split-objects on Windows..

Yes, I agree that this is the right way forward. This also means that we can simply remove split-objs, since Windows is AFAIK the only platform that still used it.

Thu, Jul 5, 2:26 AM
tdammers accepted D4920: Fix #15331 with careful blasts of parenthesizeHsType.
Thu, Jul 5, 2:24 AM
tdammers requested changes to D4930: When -odir is specified, put the object file of a C-source file into the top level odir directory (#14025).

Looks like the build fails because it can't find some .o files. Might want to look into that.

Thu, Jul 5, 2:20 AM

Wed, Jul 4

tdammers added a comment to D4847: UniqFM: Use foldl' instead of fold.

Maybe check out how this relates to D4929? There doesn't seem to be any overlap between the two patches, but D4929 puts foldl' in GHC.Prelude, which make the Data.List import redundant here.

Wed, Jul 4, 3:22 PM
tdammers requested changes to D4929: Replace most occurences of foldl with foldl'..

Would it be possible to see the performance improvements reflected in the test suite somewhere? E.g., by adding a test that compiles a program that would benefit a lot from this in terms of compilation time?

Wed, Jul 4, 10:59 AM
tdammers requested changes to D4931: Fixes an issue with deprecated symbols that are explicitly imported not throwing a warning.
  • Please fix the parser error / build failure
  • A more descriptive commit message would be great
Wed, Jul 4, 10:43 AM

Thu, Jun 21

tdammers requested changes to D4881: Rename literal constructors.

Apart from the stylistic remarks; I wonder whether this change would pull its weight. The Mach- prefix doesn't seem entirely outrageous to me (after all, these are things that are supposed to be "close to the metal" or "in machine format"), and changing them has a bit of an impact wrt future patches, merging, etc. But I really can't judge which way the balance would tip.

Thu, Jun 21, 10:47 AM
tdammers requested changes to D4872: Fix space leaks.

What about those failing CI tests? I can't make perfect sense of it, but IIUC, one test now allocates 16% less (which is to be expected, I think?), while another test now fails.

Thu, Jun 21, 10:26 AM

Mon, Jun 18

tdammers retitled D4868: Rewrite fasta in more idiomatic Haskell from Summary: Rewrite fasta in more idiomatic Haskell to Rewrite fasta in more idiomatic Haskell.
Mon, Jun 18, 6:59 AM
tdammers created D4868: Rewrite fasta in more idiomatic Haskell.
Mon, Jun 18, 6:42 AM

Jun 13 2018

tdammers added a comment to D4806: --show-iface: Always qualify names.

I think this deserves a proper discussion in a Trac issue, with some background information and concrete motivating examples.

Jun 13 2018, 4:54 AM
tdammers added a comment to D4820: Inline the default `enumFromTo` and `enumFromThenTo` methods in `Enum` typeclass..

Thanks for doing this!

The lack of INLINE breaks stream fusion, leading to degeneration in performance for Int32, Int64 and similar types

Do you understand *why* the lack of INLINE breaks fusion. It's be really good to give a little example to illustrate the steps, and where they break. Otherwise it's too much like alchemy... "try it and see".

Also could you add this info in a Note? Otherwise someone in 5 years time might wonder what those INLINE pragmas are doing, and remove them. (Yes, if they are careful enough they could do a git-annotate, but it's not so clear.)

Jun 13 2018, 4:52 AM
tdammers added a comment to D4827: Make Control.Exception.throw levity polymorphic..

I am a bit puzzled by the CI failure. I don't get why the runner cannot run ./validate.

Did I mess something up here?

Jun 13 2018, 3:18 AM
tdammers requested changes to D4831: Do not skip conc004 in GHCi way.

Is there some ticket associated with this? Do we know the context in which it allocated too much? *Someone* has clearly run into this in the past, and before undoing the "fix", I'd like some assertion that the problem has actually gone away, rather than us just not running it in the circumstances that trigger it.

Jun 13 2018, 3:06 AM
tdammers accepted D4838: testsuite: Fix T4442 on i386.
Jun 13 2018, 2:28 AM
tdammers accepted D4830: Disable `-fdefer-out-of-scope-variables` in ghci..

Fantastic, thanks a lot.

Jun 13 2018, 2:03 AM

Jun 12 2018

tdammers abandoned D4833: Disable error deferring in interactive statements.

Abandoning in favor of D4830.

Jun 12 2018, 10:16 AM
tdammers requested changes to D4830: Disable `-fdefer-out-of-scope-variables` in ghci..

This solves the same problem as D4833, but seems like a more elegant solution, so I suggest we roll with this patch instead of mine.

Jun 12 2018, 10:16 AM
tdammers added a comment to D4833: Disable error deferring in interactive statements.

@tdammers We have already disabled -fdefer-type-errors and -fdefer-typed-holes in GHCi (by Phab:D1527), see https://github.com/ghc/ghc/blob/master/compiler/typecheck/TcRnDriver.hs#L2070. I opened a ticket Trac #15259 yesterday to disable -fdefer-out-of-scope-variables as well, and fixed it by Phab:D4830. After doing that, the program given in Trac #14963 prints the result correctly and doesn't panic.

Jun 12 2018, 10:16 AM
tdammers added a comment to D4833: Disable error deferring in interactive statements.

I think it's ok to allow -fdefer-type-errors in files to be load to ghci, which wouldn't cause a panic and is very helpful to develop and try partial finished programs. We only need to disable -fdefer-type-errors (as well as the other two -fdefer) to interactive inputs in ghci.

Jun 12 2018, 9:51 AM
tdammers updated the diff for D4833: Disable error deferring in interactive statements.
  • Disable error deferring in interactive statements
  • Documentation
  • Test for Trac #14963 workaround
  • Updated test output
Jun 12 2018, 9:51 AM
tdammers updated the diff for D4833: Disable error deferring in interactive statements.
  • Override interactive dynflags, not session
  • Updated test output
Jun 12 2018, 9:51 AM
tdammers created D4833: Disable error deferring in interactive statements.
Jun 12 2018, 5:44 AM

May 31 2018

tdammers added inline comments to D4755: Conservatively estimate levity in worker/wrapper.
May 31 2018, 3:39 AM
tdammers requested changes to D4755: Conservatively estimate levity in worker/wrapper.
May 31 2018, 3:39 AM
tdammers added a comment to D4740: Check if both branches of an Cmm if have the same target..

Your test is failing on OS X; seems to be a difference in how the wc implementations format their output - the Linux one left-aligns the 0, while the OS X one pads it with spaces. A not very elegant, but effective, solution would be to throw a quick sed job into the pipeling, something like | sed -e 's/^\s*//' maybe?

May 31 2018, 2:25 AM

May 30 2018

tdammers requested changes to D4740: Check if both branches of an Cmm if have the same target..

Might be confusing for an unsuspecting reader to read hints about swapping the branches of a conditional, and then seeing code that also reduces conditionals down to just the plain branches.

May 30 2018, 10:50 AM
tdammers accepted D4742: Unpack and simplify QSem.
May 30 2018, 10:46 AM
tdammers updated the diff for D4744: Turn "inaccessible code" error into a warning.

Updated to reflect changed output in tests, and adding -Werror to tests that should fail.

May 30 2018, 9:45 AM
tdammers added inline comments to D4751: Add some comments about interruptibility for Chan.
May 30 2018, 9:43 AM
tdammers requested changes to D4751: Add some comments about interruptibility for Chan.
May 30 2018, 9:43 AM

May 29 2018

tdammers requested changes to D4746: Fix unparseable pretty-printing of promoted data cons.

Marking as "request changes", because this is clearly WIP.

May 29 2018, 2:57 AM
tdammers added a comment to D4745: [testing] testsuite: Accept new output.

Do we know what causes these? Shouldn't we have a ticket about this?

May 29 2018, 2:29 AM

May 28 2018

tdammers added inline comments to D4739: Fix "redundant constraint" warnings when meets functional dependencies.
May 28 2018, 8:48 AM
tdammers added inline comments to D4742: Unpack and simplify QSem.
May 28 2018, 8:35 AM
tdammers created D4744: Turn "inaccessible code" error into a warning.
May 28 2018, 3:05 AM
tdammers updated the diff for D4729: Handle abi-depends correctly in ghc-pkg.

ghc-pkg: properly recompute abi-depends for updated packages.

May 28 2018, 2:28 AM

May 24 2018

tdammers added inline comments to D4729: Handle abi-depends correctly in ghc-pkg.
May 24 2018, 1:17 PM
tdammers created D4729: Handle abi-depends correctly in ghc-pkg.
May 24 2018, 6:29 AM

May 17 2018

tdammers added a comment to D4637: Merge FUN_STATIC closure with its SRT.

Ömer suggested that this build error may be related: https://gist.github.com/tdammers/259593ba48374fa1b3e0885ada646d08

May 17 2018, 3:30 AM

Apr 30 2018

tdammers updated the diff for D4635: Fix performance regressions from #14737.

Fix as per Trac #15019.

Apr 30 2018, 4:36 AM
tdammers added inline comments to D4635: Fix performance regressions from #14737.
Apr 30 2018, 2:36 AM

Apr 26 2018

tdammers requested review of D4635: Fix performance regressions from #14737.
Apr 26 2018, 11:12 AM

Apr 20 2018

tdammers updated the diff for D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
  • Add SCCs to hunt down Trac #14683
  • Add regression test (Grammar.hs from Trac #14683)
  • Improve coercionKind(Role) perfomance
  • Cache coercion roles in NthCo
Apr 20 2018, 4:01 AM

Apr 19 2018

tdammers updated the summary of D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
Apr 19 2018, 9:28 AM

Apr 16 2018

tdammers requested review of D4568: Remove unnecessary check in simplCast.
Apr 16 2018, 2:54 AM

Apr 12 2018

tdammers updated the diff for D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
  • Added SCCs to hunt down Trac #14683
  • Improve coercionKind(Role) perfomance
  • Cache coercion roles in NthCo
Apr 12 2018, 10:43 AM

Apr 11 2018

tdammers updated the diff for D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
  • Bump perf test targets
Apr 11 2018, 6:52 AM

Apr 5 2018

tdammers added a comment to D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.

Separate patch with "debug only" improvements in D4570.

Apr 5 2018, 6:44 AM
tdammers updated the diff for D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
  • Fix bug in invocation of decomposeFunCo
Apr 5 2018, 6:43 AM
tdammers added a comment to D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.

I have fixed the bug and pushed to wip/tdammers/D4394

Apr 5 2018, 6:24 AM
tdammers updated the summary of D4395: Discard reflexive casts during Simplify.
Apr 5 2018, 5:13 AM
tdammers updated the diff for D4395: Discard reflexive casts during Simplify.

Rewrote commit message & implemented suggested change.

Apr 5 2018, 5:12 AM
tdammers abandoned D4569: Simplify simplCast.
Apr 5 2018, 5:12 AM

Apr 3 2018

tdammers added a comment to D4395: Discard reflexive casts during Simplify.

@simonpj, could you quickly review & comment on the updated summary (identical to commit message)?

Apr 3 2018, 10:30 AM
tdammers updated the summary of D4395: Discard reflexive casts during Simplify.
Apr 3 2018, 10:29 AM
tdammers added a comment to D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.

Oh, and BTW, this is D4394 and has its own branch, wip/tdammers/D4394. The error can be reproduced by running validate on this branch.

Apr 3 2018, 8:20 AM
tdammers added a comment to D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.

I suppose I can check out wip/tdammers/D4395-new, build it, and try to fix it, but in what way does it not work?

Apr 3 2018, 8:17 AM
tdammers added a comment to D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.

Apparently, the way I tried to fix rebasing-induced errors wasn't correct, but unfortunately I don't understand the code well enough to see where I went wrong. Maybe @goldfire or @simonpj could take a peek?

Apr 3 2018, 5:51 AM
tdammers updated the diff for D4395: Discard reflexive casts during Simplify.

Discard reflexive casts during Simplify

Apr 3 2018, 5:50 AM

Mar 30 2018

tdammers updated the diff for D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
  • Fix rebasing-induced failures
Mar 30 2018, 6:23 AM
tdammers added a comment to D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.

More specifically, I broke something, but e3dbb44f53b2f9403d20d84e27f187062755a089 also did - that commit (currently in master) removes the nextRole export from TcType.hs, which this patch relies on.

Mar 30 2018, 2:47 AM

Mar 29 2018

tdammers added a comment to D4395: Discard reflexive casts during Simplify.

Wait a tick. Before your most recent commits, did addCoerce check isReflexiveCo twice? That's ridiculous (and my fault, I'm sure). That could be the reason for the slowdown.

Mar 29 2018, 7:41 AM
tdammers added a comment to D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.

Looks like I broke something when rebasing onto master.

Mar 29 2018, 7:38 AM

Mar 27 2018

tdammers updated the diff for D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
  • Fixed NthCo Note (discussion in D4385)
  • Documented invariant on cached roles
  • Tighten cached role in NthCo
  • NthCo handling, assertions and documentation improvements
  • Fixes to Coercion.hs
  • Removed dead code
  • Refactored setNominalRole_maybe to avoid dragging role through recursion
  • Another fix as per D4394
  • Fixed unused variables
  • Refactor mkCoCast
  • Fix test to reflect changed NthCo ctor
  • Increased allocation limit for T5321 on OS X
  • Fix rebasing mistake
Mar 27 2018, 10:17 AM
tdammers updated the diff for D4395: Discard reflexive casts during Simplify.
  • Discard reflexive casts during Simplify
  • Fix huge performance regression
Mar 27 2018, 9:57 AM

Mar 26 2018

tdammers updated the diff for D4395: Discard reflexive casts during Simplify.
  • Fix huge performance regression
Mar 26 2018, 6:54 AM
tdammers added a comment to D4395: Discard reflexive casts during Simplify.

On further thought, I think it'd be better to get this landed first, and then look into further improvements.

Mar 26 2018, 6:54 AM
tdammers added a comment to D4395: Discard reflexive casts during Simplify.

Are you saying that, *with the code I copy/pasted in the previous comment*, the perf is actually *a lot worse* than before the patch? That seems very implausible; it's such a benign change. (But NB you must use the code I copy/pasted. Your patch has isReflexive twice.)

Mar 26 2018, 6:54 AM
tdammers added a comment to D4395: Discard reflexive casts during Simplify.

So it does indeed look like simplCast is still the culprit. I can re-run with the SCC added back in if you need solid proof (though this is rather time consuming).

Are you saying that, *with the code I copy/pasted in the previous comment*, the perf is actually *a lot worse* than before the patch? That seems very implausible; it's such a benign change. (But NB you must use the code I copy/pasted. Your patch has isReflexive twice.)

Mar 26 2018, 4:45 AM
tdammers added a comment to D4395: Discard reflexive casts during Simplify.

But if it doesn't fix the problem, is simplCast still at the top of allocation list? And where is that time going?

Mar 26 2018, 2:35 AM
tdammers added a comment to D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.

Where is the data that says this patch degrades performance? The data in the previous comment shows a huge improvement.

Mar 26 2018, 2:35 AM

Mar 22 2018

tdammers updated the diff for D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.

Cache coercion roles in NthCo and refactor coercionKindsRole

Mar 22 2018, 7:11 AM

Mar 21 2018

tdammers added a comment to D4395: Discard reflexive casts during Simplify.

Hold back the horses, I've done some additional profiling, "just to make sure", and it turns out that, at least for the Grammar.hs test case that tripped this whole thing off, performance is actually getting over an order of magnitude *worse*, compared to GHC HEAD. I don't think we should land this just yet.

Mar 21 2018, 12:23 PM

Mar 20 2018

tdammers updated the diff for D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
  • Fix rebasing mistake
Mar 20 2018, 11:46 AM
tdammers added a comment to D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.

This should apply cleanly onto GHC HEAD (as of today)

Mar 20 2018, 7:02 AM
tdammers updated the diff for D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
  • Fixed NthCo Note (discussion in D4385)
  • Documented invariant on cached roles
  • Tighten cached role in NthCo
  • NthCo handling, assertions and documentation improvements
  • Fixes to Coercion.hs
  • Removed dead code
  • Refactored setNominalRole_maybe to avoid dragging role through recursion
  • Another fix as per D4394
  • Fixed unused variables
  • Refactor mkCoCast
  • Fix test to reflect changed NthCo ctor
  • Increased allocation limit for T5321 on OS X
Mar 20 2018, 7:01 AM
tdammers added inline comments to D4395: Discard reflexive casts during Simplify.
Mar 20 2018, 6:33 AM
tdammers updated the diff for D4395: Discard reflexive casts during Simplify.

Squash & remove commits that don't belong here

Mar 20 2018, 5:59 AM

Mar 19 2018

tdammers added a comment to D4395: Discard reflexive casts during Simplify.

The git history seems confused here, which is perhaps my fault. I contributed the code in CoreOpt.hs and Simplify.hs, which is relevant for this patch. The other changes are unrelated (and are around caching NthCo roles, which is a separate optimization).

Mar 19 2018, 11:07 AM

Mar 17 2018

tdammers updated the diff for D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
  • Increased allocation limit for T5321 on OS X
Mar 17 2018, 12:05 PM

Mar 16 2018

tdammers added a comment to D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.

when I compile the relevant file with HEAD and the D4394 version, we see the massive improvement we expect

Are you saying that the 5.5% allocation lossage shows up when you run the testsuite, but NOT when you run this test individually? That really is mysterious. Or are you saying something bad only happens on OSX which you do not have access to?

Mar 16 2018, 3:00 AM

Mar 15 2018

tdammers added a comment to D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.

Oh bother!

That does come as a surprise. I really thought everything would be faster.

Mar 15 2018, 11:25 AM
tdammers added a comment to D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.

Hmm, one test now fails on OS X. The relevant test case T5321 allocates 5.5% more now, which is 0.5% more than our set limit of 5%. The build also succeeds on the other platforms, only OS X seems to be having trouble.

Mar 15 2018, 7:54 AM

Mar 14 2018

tdammers added a comment to D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.

Rebased onto current master.

Mar 14 2018, 5:42 AM
tdammers updated the diff for D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
  • Fixed NthCo Note (discussion in D4385)
  • Documented invariant on cached roles
  • Tighten cached role in NthCo
  • NthCo handling, assertions and documentation improvements
  • Fixes to Coercion.hs
  • Removed dead code
  • Refactored setNominalRole_maybe to avoid dragging role through recursion
  • Another fix as per D4394
  • Fixed unused variables
  • Refactor mkCoCast
  • Fix test to reflect changed NthCo ctor
Mar 14 2018, 4:57 AM

Mar 13 2018

tdammers updated the diff for D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
  • Fix test to reflect changed NthCo ctor
Mar 13 2018, 8:09 AM

Mar 6 2018

tdammers updated the diff for D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
  • Refactor mkCoCast
Mar 6 2018, 5:19 AM

Mar 5 2018

tdammers updated the summary of D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
Mar 5 2018, 10:40 AM
tdammers added inline comments to D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
Mar 5 2018, 8:28 AM
tdammers updated the diff for D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
  • Fix unused variables
Mar 5 2018, 8:28 AM
tdammers added inline comments to D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
Mar 5 2018, 8:28 AM
tdammers updated the summary of D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.
Mar 5 2018, 7:04 AM
tdammers updated the diff for D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.

Address review remarks

Mar 5 2018, 6:03 AM

Feb 28 2018

tdammers added a comment to D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.

do you want to land this diff as-is, or do you want to address the issues noted first?

I meant the latter, assuming you are referring to my comments immediately above. They are all easy to deal with.

Feb 28 2018, 10:00 AM

Feb 27 2018

tdammers added a comment to D4394: Caching coercion roles in NthCo and coercionKindsRole refactoring.

@simonpj I'm a bit confused; do you want to land this diff as-is, or do you want to address the issues noted first?

Feb 27 2018, 5:35 PM