D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 17736 - bigint opunary should be better performing
Summary: bigint opunary should be better performing
Status: RESOLVED INVALID
Alias: None
Product: D
Classification: Unclassified
Component: phobos (show other issues)
Version: D2
Hardware: All All
: P1 enhancement
Assignee: No Owner
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-08-10 01:07 UTC by Steven Schveighoffer
Modified: 2017-08-11 21:00 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Steven Schveighoffer 2017-08-10 01:07:44 UTC
Currently, opUnary!"++" maps to 
this = this + 1

However, this is going to allocate a whole new temporary structure and then throw it away. opUnary ++ and -- should edit the bigint in place.

Same goes for opOpAssign, those could be improved, but this issue is for the low hanging fruit, as incrementing is easy.
Comment 1 hsteoh 2017-08-10 01:25:35 UTC
I notice also that binary operators on BigInt (even the opOpAssign ones like +=) will create new instances of BigInt rather than update in-place.

One trouble with updating in-place is that it makes BigInt assignment either expensive (always copy) or exhibit reference semantics:

---
BigInt x = 1;
BigInt y = x;
++y;
writeln(x); // will this print 1 or 2?
---

If I understand the BigInt design correctly, reference semantics are *not* desirable because we want BigInt to be more-or-less a drop-in replacement of fixed-size ints, and lots of code will break in subtle ways if the by-value semantics was substituted with by-reference semantics.

One thought is that if BigInt uses some sort of reference-counting scheme, then if the refcount is 1 we can update in-place, otherwise allocate a new BigInt to hold the result as usual.
Comment 2 Steven Schveighoffer 2017-08-10 01:34:47 UTC
Oh, so assignment just rebinds to the existing data? Then this request is invalid.

One thing we could do is make a MutableBigInt, that is allowed to modify itself. But that's a much bigger project.
Comment 3 hsteoh 2017-08-10 16:07:22 UTC
IMO, the refcounting idea is still valid, if a bit more complicated to implement. It would be important for reducing GC load on BigInt-heavy code, I think.
Comment 4 Steven Schveighoffer 2017-08-10 16:10:05 UTC
Either way, this is not a "simple" enhancement. But feel free to take over this enhancement request if you want to write it up.
Comment 5 hsteoh 2017-08-11 20:59:26 UTC
Wrote it up here:

https://issues.dlang.org/show_bug.cgi?id=17746

Doesn't necessarily mean I have the time to actually implement this, though. :-)