Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#11568 closed bug (fixed)

Regression in nofib/shootout/k-nucleotide

Reported by: simonmar Owned by: lukemaurer
Priority: high Milestone: 8.2.1
Component: Compiler Version: 7.10.3
Keywords: JoinPoints Cc: maurerl@…, pdownen@…, ariola@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D2853
Wiki Page:

Description

Fixing the sizeExpr bug in #11564 caused a regression in nofib/shootout/k-nucleotide.

We don't know yet exactly which function was affected. See Phab:D1900 for details and some -ddump-simpl and -ticky dumps. We should investigate this before merging Phab:D1900 into a release.

Attachments (2)

Main.dump-stg.gz (110.5 KB) - added by dfeuer 3 years ago.
STG dump for HEAD c. Jan 8 2017
headallocd.ticky (10.2 KB) - added by dfeuer 3 years ago.
ticky-ticky profile for HEAD c. Jan 8, 2017

Download all attachments as: .zip

Change History (16)

comment:1 Changed 4 years ago by thomie

perf.haskell.org reports the following numbers as well:

Benchmark name	                previous	change	        now	
tests/alloc/T3064 	        297834384	+ 3.18%	        307311896	bytes
tests/alloc/T5631 	        1164415992	+ 108.27%	2425081664	bytes
tests/alloc/T7257         	1654893352	- 14.50%	1414893352	bytes
tests/alloc/T783         	489042024	+ 3.47%	        505987776	bytes
tests/alloc/T9020         	703211744	+ 16.45%	818924416	bytes
tests/alloc/T9872d         	535824528	+ 3.09%	        552399688	bytes
tests/alloc/T9961         	731573736	+ 5.10%	        768861280	bytes
tests/alloc/haddock.Cabal 	10472403176	+ 4.52%	        10945830216	bytes
tests/alloc/haddock.compiler 	59679619080	+ 4.56%	        62402376816	bytes

Links to: #3064, #5631, #783, #9020, #9872, #9961.

comment:2 Changed 3 years ago by bgamari

Type of failure: None/UnknownRuntime performance bug

comment:3 Changed 3 years ago by bgamari

Owner: set to dfeuer

David is currently looking into this.

comment:4 Changed 3 years ago by dfeuer

According to ticky-ticky profiling, the big things being allocated (alloc'd) after the patch that caused the regression are $j1 and $j2 in $s$wfoldMe. Neither of these showed up in ticky-ticky before. I think $j2 may be new after the patch, but $j1 was there before; perhaps it was LNEed before and isn't now? I'll try to dig deeper on Sunday.

Changed 3 years ago by dfeuer

Attachment: Main.dump-stg.gz added

STG dump for HEAD c. Jan 8 2017

Changed 3 years ago by dfeuer

Attachment: headallocd.ticky added

ticky-ticky profile for HEAD c. Jan 8, 2017

comment:5 Changed 3 years ago by dfeuer

I'm seeing a lot of allocation of j2_sbYy. It seems this is not let-no-escape because it's used in non-tail position. But every use site looks identical (modulo distinctions that at least shouldn't matter). Specifically, an application of j2_sbYy is case-matched, and the non-void# portion of the result is passed to $j3_sbZZ. Something must be preventing GHC from seeing that j2_sbYy should itself call $j3_sbZZ.

comment:6 Changed 3 years ago by simonpj

Aha. Yes, j2_sbYy was born as a join-point but some subsequent transformation changed it into a non-join point.

Happily Luke's work on join points should nail this. I suggest you down-tools on this until Luke has his stuff committed.

comment:7 Changed 3 years ago by simonpj

Keywords: JoinPoints added

comment:8 Changed 3 years ago by simonpj

Cc: maurerl@… pdownen@… ariola@… added

comment:9 Changed 3 years ago by dfeuer

Blocked By: 12988 added

comment:10 Changed 3 years ago by lukemaurer

Looks to me like Simon's instinct was correct. Every instance of $j1 remains a join point now, since those pesky contexts that were getting put around the calls are getting put into $j1 instead.

In fact, k-nucleotide is one of the poster children for join points—it gets an 86.0% decrease in allocs.

comment:11 Changed 3 years ago by dfeuer

Differential Rev(s): Phab:D2853
Owner: changed from dfeuer to lukemaurer

comment:12 Changed 3 years ago by dfeuer

Blocked By: 12988 removed

comment:13 Changed 3 years ago by dfeuer

Blocked By: 12988 added
Resolution: fixed
Status: newclosed

Luke's work fixed this in 8d5cf8bf584fd4849917c29d82dcf46ee75dd035. Confirmed by Gipeda.

comment:14 Changed 3 years ago by simonpj

Blocked By: 12988 removed
Note: See TracTickets for help on using tickets.