Java Units library memory warning

[agasser]

Although I don't disagree with the idea of mutable versions of the measures classes, I question the wording in the documentation. It recommends against creating new instances of the objects on every robot iteration. This warning doesn't exist for other classes that we create on every period; such as ChassisSpeeds, Translation2d, Rotation2d, and even boxing like Double. Does this warning really belong in the documentation?

[agasser]

This came to mind as I was trying out the units library. My teleop drive command uses three DoubleSuppliers for X, Y, and Omega speed.

Today, my X supplier looks like this:

public DoubleSupplier translationX() {
  return () ->-modifyAxis(driverController.getLeftY()) * MAX_VELOCITY_METERS_PER_SECOND;
}

This creates an instance of Double on every period. It is created automatically by autoboxing.

If I update my MAX_VELOCITY_METERS_PER_SECOND constant to use Measure<Velocity<Distance>>, my supplier becomes something like this:

public Supplier<Measure<Velocity<Distance>>> translationX() {
  return () -> MAX_VELOCITY.times(-modifyAxis(driverController.getLeftY()));
}

This implementation using the units library requires creation of a Measure<Velocity<Distance>> on every iteration. It is created by the times() method.

Is the new implementation cause for concern?

[Starlight220]

This creates an instance of Double on every period. It is created automatically by autoboxing.

Why is this? The point of DoubleSupplier over Supplier is to avoid boxing.

[agasser]

Yeah, you're right, that's a bad example!

The point still holds. In fact, there are many cases of such usage in WPILib: Rotation2d, Pose2d, ChassisSpeeds, SwerveModulePosition, and SwerveModuleState. None of these classes have mutable versions. The risk of introducing mutability outweighs the theoretical benefit.