build: Annotate with units from Checker Framework #1712
Conversation
This is a proof-of-concept as I wanted to learn more about this framework.
| @SuppressWarnings("units") | ||
| public static @s(Prefix.micro) long stopwatchToMicroseconds(Stopwatch sw) { | ||
| return sw.elapsed(MICROSECONDS); | ||
| } |
There was a problem hiding this comment.
Convenience method to convert a stopwatch to a long annotated as microseconds.
| @SuppressWarnings("units") | ||
| @s(Prefix.micro) | ||
| long stallUSecs = 0; |
There was a problem hiding this comment.
Assigning constants needed a warning suppression, including in the Checker Framework examples. https://github.com/typetools/checker-framework/blob/master/docs/examples/units-extension/UnitsExtensionDemo.java
| @@ -101,7 +104,7 @@ private long runInterruptible(Stopwatch stopwatch, ResourceLimits limits) | |||
| metadata = operationContext.operation.getMetadata().unpack(ExecuteOperationMetadata.class); | |||
| } catch (InvalidProtocolBufferException e) { | |||
| log.log(Level.SEVERE, "invalid execute operation metadata", e); | |||
| return 0; | |||
| return ZERO_MICROSECONDS; | |||
There was a problem hiding this comment.
This was rather annoying as Checker Framework refused to return "unit-less" 0 constant.
(arguably, 0 probably doesn't need units as 0 seconds == 0 minutes, but I suppose 0 degrees Celsius is not the same as 0 degrees Fahrenheit)
| executionTime.observe(usecs / 1000.0); | ||
| executionStallTime.observe(stallUSecs / 1000.0); | ||
| executionTime.observe(TimeUnit.MILLISECONDS.convert(uSecs, TimeUnit.MICROSECONDS)); | ||
| executionStallTime.observe(TimeUnit.MILLISECONDS.convert(stallUSecs, TimeUnit.MICROSECONDS)); |
There was a problem hiding this comment.
This wasn't part of the CheckerFramework, but I just didn't trust myself to do unit conversions :) Can revert..
This is a proof-of-concept as I wanted to learn more about this framework.
It's a bit verbose so I'm not convinced. I didn't notice any build time degredations.
If you want to pull it and try, just switch one of those
@s(Prefix.micro)to@s(Prefix.nano)and watch thebazel build //src/...will fail.If we do like this, I can try to tidy it up by adding a convenience
@microsecondsor@us(not very read-able) Annotations per https://checkerframework.org/manual/#extending-units