Skip to content

Conversation

@SeungheonOh
Copy link
Collaborator

@SeungheonOh SeungheonOh commented Nov 4, 2025

compares NegateValue and ScaleValue builtin.

I benchmarked two implementation of NegateValue. First, with (-1) * quantity other with negate quantity and two results differ significantly.

This is result is from having (-1) * quantity

negatevsscalevalue

This one is with negate quantity

negatevsscalevalue-betternegate

where

negateQuantity :: Quantity -> Maybe Quantity
negateQuantity (UnsafeQuantity x) =
  if x == -170141183460469231731687303715884105728
  then Nothing
  else Just $ UnsafeQuantity (negate x)

I didn't know negate would perform so much better than (-1) *

@SeungheonOh SeungheonOh added Do not merge Builtins No Changelog Required Add this to skip the Changelog Check labels Nov 4, 2025
@SeungheonOh SeungheonOh self-assigned this Nov 4, 2025
@SeungheonOh
Copy link
Collaborator Author

negatevsscalevalue-betternegate-test

This is with slower negateValue:

negateQuantity :: Quantity -> Maybe Quantity
negateQuantity (UnsafeQuantity x) = quantity (negate x)
{-# INLINE negateQuantity #-}

@SeungheonOh SeungheonOh requested review from kwxm and zliu41 November 5, 2025 17:06
@zliu41
Copy link
Member

zliu41 commented Nov 5, 2025

Does any of these graphs compare two implementations that are otherwise exactly the same, except one uses negateQuantity, and the other uses scaleQuantity?

You seem to be comparing negateValue and scaleValue, but there might be other differences between them other than negation vs. multiplication?

@zliu41
Copy link
Member

zliu41 commented Nov 5, 2025

It would also be nice to verify whether 0 - x is indeed that much faster than (-1) * x (a simple criterion benchmark would do). Because according to their cost models, the differences aren't that big.

@zliu41
Copy link
Member

zliu41 commented Nov 5, 2025

In any case, I think we should still add scaleValue instead of negateValue. We can always optimize for special cases like 1 or -1 later.

@SeungheonOh
Copy link
Collaborator Author

Does any of these graphs compare two implementations that are otherwise exactly the same, except one uses negateQuantity, and the other uses scaleQuantity?

Yes!

@SeungheonOh
Copy link
Collaborator Author

We got all we needed from benchmark!

@SeungheonOh SeungheonOh closed this Nov 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Builtins Do not merge No Changelog Required Add this to skip the Changelog Check

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants