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

Performance regression (probably related to ephemerons) #7157

Closed
vicuna opened this issue Feb 26, 2016 · 7 comments
Closed

Performance regression (probably related to ephemerons) #7157

vicuna opened this issue Feb 26, 2016 · 7 comments
Milestone

Comments

@vicuna
Copy link

vicuna commented Feb 26, 2016

Original bug ID: 7157
Reporter: AltGr
Status: closed (set by @damiendoligez on 2016-03-03T12:48:38Z)
Resolution: open
Priority: normal
Severity: major
Platform: amd64
OS: Linux
Target version: 4.03.0+dev / +beta1
Fixed in version: 4.03.0+dev / +beta1
Category: runtime system and C interface
Related to: #7161
Monitored by: bobot

Bug description

We have a bench on which it seems merging the Ephemerons branch may have had large effects (judging by the date when the slowdown appeared, I might be wrong)

http://bench.flambda.ocamlpro.com/graph?bench=numal-fft

The code is at https://github.com/AltGr/ocaml-numerical-analysis/blob/bench/fft/bench.ml ; may it be caused by the Gc.set ?

Here is a more detailed view of the comparison between Jan. 26th and Jan. 27th versions:

http://bench.flambda.ocamlpro.com/compare?test=2016-01-27-2300%2Ftrunk%2Bbench&reference=2016-01-26-2300%2Ftrunk%2Bbench

We get +60% time, +245% major words on this bench.

Originally reported at #22 (comment)

File attachments

@vicuna
Copy link
Author

vicuna commented Feb 26, 2016

Comment author: @alainfrisch

           WITH Gc.set   WITHOUT Gc.set
BEFORE        2.2          3.6
AFTER         3.0          3.6

Tweaking Gc.space_overhead does not seem to impact performance (neither after nor before).

Now, comparing BEFORE/AFTER for various values of minor_heap_size:

             BEFORE    AFTER
 s=4M          3.5      3.7
 s=8M          3.1      3.5
 s=16M         2.6      3.5
 s=32M         2.1      2.3
 s=64M         2.1      2.1
 s=128M        2.0      2.0

Now the result of OCAMLRUNPARAM=s=16M,v=0x400:

BEFORE
allocated_words: 445645203
minor_words: 440402317
promoted_words: 146800948
major_words: 152043834
minor_collections: 28
major_collections: 4
heap_words: 88033280
heap_chunks: 28
top_heap_words: 88033280
compactions: 0


AFTER
allocated_words: 445645364
minor_words: 440402478
promoted_words: 222822724
major_words: 228065610
minor_collections: 86
major_collections: 8
heap_words: 57882624
heap_chunks: 25
top_heap_words: 101238784
compactions: 1

@vicuna
Copy link
Author

vicuna commented Feb 27, 2016

Comment author: bobot

Alain, does BEFORE/AFTER corresponds to before and after the merge commit?

@vicuna
Copy link
Author

vicuna commented Feb 27, 2016

Comment author: bobot

The commits of the ephemerons' branch should all compile. Particularly if you take the two first commits that contains initial factorization before any ephemerons related modifications:

commit d94f1da32f75793c2dee162df82990ca6f4adaa4
Author: doligez <damien.doligez@inria.fr>
Date:   Wed Apr 2 13:46:19 2014 +0200

    [GC] Factorize the management of resizable arrays
    
       For major to minor pointers

commit 3b6677096675d8a4c1ec3cf19d81cf788847be2d
Author: François Bobot <francois.bobot@cea.fr>
Date:   Sun Feb 23 15:51:27 2014 +0100

    [GC] Simplify mark_slice function

On fft I obtain:

  • 1.85 before the merge
  • 1.85 3b66770 (first commit of the branch)
  • 2.40 d94f1da (second commit of the branch)
  • 2.42 after the merge

(I have not been able to get real results with operf-macro so I used a crude /usr/bin/time /bin/dash -c "for i in \$(seq 1 50); do ./fft.opt; done")

So I think that d94f1da is the culprit. However I don't yet understand exactly why.

@vicuna
Copy link
Author

vicuna commented Feb 29, 2016

Comment author: @mshinwell

I think I see what is wrong with it. I will make a GPR.

@vicuna
Copy link
Author

vicuna commented Feb 29, 2016

Comment author: @alainfrisch

I think the problem is in the use of "void" instead of "value*" for the definition of the generic table. It changes the semantics of pointer arithmetic in alloc_generic_table, e.g. "tbl->base + tbl->size" adds tbl->size bytes when the pointer has type void*, vs tbl->sizesizeof(value) when the pointer has type value**. I've attached a patch.

@vicuna
Copy link
Author

vicuna commented Feb 29, 2016

Comment author: @mshinwell

#488

@vicuna
Copy link
Author

vicuna commented Mar 3, 2016

Comment author: @damiendoligez

Fixed in 4.03 (commit 6e223fd).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant