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

perf: use a monomorphic impl for CCMonomorphic.{min,max} #453

Merged
merged 1 commit into from
Jul 9, 2024

Conversation

c-cube
Copy link
Owner

@c-cube c-cube commented Jun 3, 2024

close #452

@FardaleM
Copy link
Collaborator

FardaleM commented Jun 3, 2024

From my testing, it seems enough to define min as:

let min (x:int) (y:int) = if x <= y then x else y

But my testing was with flambda enabled. Something that might be enough also is to do:

let min (x:int) (y:int) = Stdlib.min x y

But I did not try it to see if this is enough.
With both solution we would not need the conditional compilation.

@c-cube
Copy link
Owner Author

c-cube commented Jun 3, 2024

let min (x:int) (y:int) = Stdlib.min x y

I've never seen before this be faster than let min = Stdlib.min? :/
if it is, we have a problem cause this is also how compare, etc are done. But anyway, I'd rather delegate to Int.min since it might become an intrinsic in the future.

@FardaleM
Copy link
Collaborator

FardaleM commented Jun 4, 2024

I would need to look how it is compiled. The question is, does the compiler specialised the compare function or not. If not, then it goes through the polymorphic compare which is slow. That's how I found it, the min function was calling the runtime C function on my case. We would need to check if this is the case also for the other functions. But I don't know how to check that.

@FardaleM
Copy link
Collaborator

FardaleM commented Jun 4, 2024

I tried to play to see the case for min, but I don't see the function, so I don't know how to interpret this.

@c-cube
Copy link
Owner Author

c-cube commented Jun 5, 2024

it might be because polymorphic min isn't in the compiled code, it's part of the runtime.

@c-cube c-cube merged commit 60bd3ae into main Jul 9, 2024
7 checks passed
@c-cube c-cube deleted the simon/faster-minmax branch July 9, 2024 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Monomorphic.min function is slow
2 participants