tillrohrmann commented on a change in pull request #9760:
[FLINK-13982][runtime] Implement memory calculation logics
URL: https://github.com/apache/flink/pull/9760#discussion_r329051310
##########
File path:
flink-core/src/main/java/org/apache/flink/configuration/MemorySize.java
##########
@@ -115,6 +115,27 @@ public String toString() {
return bytes + " bytes";
}
+ //
------------------------------------------------------------------------
+ // Calculations
+ //
------------------------------------------------------------------------
+
+ public MemorySize add(MemorySize that) {
+ return new MemorySize(Math.addExact(this.bytes, that.bytes));
+ }
+
+ public MemorySize subtract(MemorySize that) {
+ return new MemorySize(Math.subtractExact(this.bytes,
that.bytes));
+ }
+
+ public MemorySize multiply(double multiplier) {
+ checkArgument(multiplier >= 0, "multiplier must be >= 0");
+ double product = this.bytes * multiplier;
Review comment:
I think the error can be up to 9 bits
```
final long value = Long.MAX_VALUE - 511;
final double result = value * 1.0;
final long longresult = (long) result;
System.out.println(value);
System.out.println(result);
System.out.println(longresult);
System.out.println(Math.abs(longresult - value)); // 511
```
Here one can see that the difference is `511`. Hence it is not only a single
byte in difference. Moreover, the result can be bigger than the input if
multiplied by `1.0`.
However, it should be save to multiple long values which only use the first
52 bits due to the mantissa of a `double`. Maybe one could add a checkstate or
we use `BigDecimal` to be on the safe side. Given that this path should not be
taken a lot it could be a good safety net.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
