Thank you for the review, Haohui. I'm going to commit this.
I wonder, do you have any plans to make similar changes on the methods in AclTransformation?
I think there is less reason to use ImmutableList inside AclTransformation. For the input ACL spec, the first thing we need to do is sort the entries so that the rest of the algorithms can assume the ordering. Right now, we do that as an in-place sort on the list parsed from the protocol, so it's convenient for that to remain mutable. For the output calculated ACL, it's always the full logical ACL. For the physical storage, we always end up unpacking a few of those entries for mapping onto the permission bits, and the final list stored in the AclFeature is a different, smaller list. I think the change is really important inside AclStorage and AclFeature though to guarantee that there is no accidental mutation of an ACL associated to an existing inode.
BTW, I just learned that the Guava immutable collections have a smaller memory footprint than the typical JDK data structures:
That's a nice side benefit of this patch, and it's worth considering for future changes in areas that consume a lot of memory.