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

Operation result being copy/noncopy is inconsistent for some operations #48

Open
Razenpok opened this issue Apr 5, 2019 · 1 comment
Labels
is this optimal? possible performance improvements

Comments

@Razenpok
Copy link
Collaborator

Razenpok commented Apr 5, 2019

I haven't checked other operations, but in some cases add can return the same instance that was passed as left-hand side Decimal.
https://github.com/Patashu/break_infinity.js/blob/master/src/break_infinity.ts#L869

We need to decide if we want Decimal to be

  • Faster, with less allocations
  • Slower, but without pitfalls

I believe that returning the same instance can lead to some quirky bugs caused by innate mutability of Decimal, but it's your call.
Keep in mind that "faster" and "slower" doesn't really mean object allocation itself (which a really lightweight operation), but instead the fact that it will add to garbage and GC will be invoked sooner.

@Patashu
Copy link
Owner

Patashu commented Apr 5, 2019

As you say the less allocations, the better. We're going for speed here (as much speed as we can get in JS anyway...)

Directly mutating break_infinity.js is allowed, but it shouldn't be a common operation to do say c = Decimal.add(a, b); c.e += 1; or whatever, that would actually be affected by this. I've never heard of anyone being surprised by the behaviour you list.

The main reason to not go full allocationless and mutate arguments is that THAT would be too surprising (if you add or mul what you thought was a constant and break_infinity.js mutates it, that requires you to keep reallocating or cloning your constant, defeating the purpose).

@Patashu Patashu added the is this optimal? possible performance improvements label Apr 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is this optimal? possible performance improvements
Projects
None yet
Development

No branches or pull requests

2 participants