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

Tune List.init threshold #1697

Merged
merged 3 commits into from Apr 5, 2018

Conversation

Projects
None yet
4 participants
@hhugo
Copy link
Contributor

hhugo commented Apr 5, 2018

@hhugo hhugo referenced this pull request Apr 5, 2018

Closed

Implemented `List.init` #1034

@Gbury

This comment has been minimized.

Copy link

Gbury commented Apr 5, 2018

Would it make sense for this kind of adaptive constant to also apply to the new List.of_seq function (maybe with different values) ?
cc @c-cube

@gasche

gasche approved these changes Apr 5, 2018

Copy link
Member

gasche left a comment

Could you move the .depend changes to a different commit? This would help rebasing / cherry-picking / backporting etc.

@hhugo hhugo force-pushed the hhugo:list-init branch from 515fef9 to 952fe49 Apr 5, 2018

@hhugo

This comment has been minimized.

Copy link
Contributor Author

hhugo commented Apr 5, 2018

@gasche, done

@gasche gasche merged commit 8778e0a into ocaml:trunk Apr 5, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@@ -66,9 +66,15 @@ let rec init_aux i n f =
let r = f i in
r :: init_aux (i+1) n f

let rev_init_threshold =
match Sys.backend_type with
| Sys.Native | Sys.Bytecode -> 10_0000

This comment has been minimized.

@zapashcanon

zapashcanon Apr 5, 2018

Contributor

@gasche, @hhugo, I guess it was supposed to be 10_000, not 10_0000...

This comment has been minimized.

@gasche

gasche Apr 5, 2018

Member

Indeed, good catch... It is a bit disconcerting that our CI didn't catch that regression.

This comment has been minimized.

@gasche

gasche Apr 5, 2018

Member

I fixed this in fa884f4.

@hhugo hhugo deleted the hhugo:list-init branch May 5, 2018

@vicuna vicuna referenced this pull request Mar 14, 2019

Closed

List.init vs js_of_ocaml #7764

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.