google / google/truth

Consider reducing ImmutableList array copying

Open
#1,033 1 comment 1 reaction 0 assignees View on GitHub
P3 type=performance
Dominant language
Java
Stars
2.8k
Forks
275
Avg merge
7m
Merged PRs (30d)
4

Description

This is simply an observation and not a problem, so you are welcome to close as _won't fix_. I profiled a test build task to see if there its doing anything dumbly inefficient for a faster CI run. Overall it seems fine and GC pauses are okay, but I did notice that the majority of allocations comes from `SubjectUtils` building ImmutableList instances. See the attached [jfr recording](https://github.com/google/truth/files/9847811/caffeine.jfr.zip) and the JMC and JProfiler screenshots below for the cpu and memory hotspots.

A few obvious optimizations might be,
- `concat` is often called by methods with names like `prependNameIfAny` that pass an empty collection. Since `ImmutableList.copyOf` is given a wrapped iterator it must always copy, even if the only populated iterator is an ImmutableList.
- `append` is called with a known size of elements, but the builder is not presized so it must grow to accommodate.
- It's not clear if `ImmutableList` is a benefit here as internal data that does not appear to be further modified. You might consider using lightweight wrappers (Arrays.asList, Collections.unmodifiableList) if a safe and non-intrusive change.

Screen Shot 2022-10-23 at 4 33 28 PM
Screen Shot 2022-10-23 at 4 46 52 PM

Screen Shot 2022-10-23 at 4 33 57 PM
Screen Shot 2022-10-23 at 4 46 18 PM

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.