photonvision icon indicating copy to clipboard operation
photonvision copied to clipboard

Set the initial capacity of ArrayLists where possible

Open kcooney opened this issue 4 months ago • 8 comments

Description

This can reduce reallocations of the underlying arrays.

Meta

Merge checklist:

  • [x] Pull Request title is short, imperative summary of proposed changes
  • [x] The description documents the what and why
  • [ ] If this PR changes behavior or adds a feature, user documentation is updated
  • [ ] If this PR touches photon-serde, all messages have been regenerated and hashes have not changed unexpectedly
  • [ ] If this PR touches configuration, this is backwards compatible with settings back to v2024.3.1
  • [ ] If this PR touches pipeline settings or anything related to data exchange, the frontend typing is updated
  • [ ] If this PR addresses a bug, a regression test for it is added

kcooney avatar Sep 01 '25 16:09 kcooney

I'd like to see some data or an argument for why this is necessary. Currently, the size of our array lists isn't really something that tends to be an issue, and it might be better to avoid the added complexity this brings for such a small beenfit.

samfreund avatar Sep 03 '25 15:09 samfreund

In my mind its small added complexity for small added benefit, seems reasonable.

spacey-sooty avatar Sep 03 '25 15:09 spacey-sooty

Only if consistently applied across the codebase. I'd rather we stick to 100% lists without a preset size, unless test data shows that this makes a statistically significant difference on our target platforms.

mcm001 avatar Sep 05 '25 04:09 mcm001

@mcm001 @samfreund I made this at the suggestion of @Gold856 (see this comment). For most of the changes in this PR, I think the added complexity is extremely low. Agreed that the added benefit is hard to measure.

I'd rather we stick to 100% lists without a preset size

Personally I think the changes in #2079 were justified. As of Java 17 the initial capacity of an ArrayList is 10¹, so if the capacity was unset then if there were more than ten queued results, PhotonCamera.getAllUnreadResults() would resize two arrays at least once each.

¹ In Java 17 ArrayList instances created via the default constructor share a common empty array; an array of size 10 would be allocated if a single element was inserted into an empty ArrayList.

kcooney avatar Sep 14 '25 15:09 kcooney

I'd still like to see some performance metrics if possible. If you see the ArrayList constructor on the hotpath on roborio or older Pi platforms, I'd be more inclined to merge.

mcm001 avatar Sep 15 '25 03:09 mcm001

I'd still like to see some performance metrics if possible. If you see the ArrayList constructor on the hotpath on roborio or older Pi platforms, I'd be more inclined to merge.

Do you have pointers on how I would get those metrics?

kcooney avatar Sep 22 '25 15:09 kcooney

I'd still like to see some performance metrics if possible. If you see the ArrayList constructor on the hotpath on roborio or older Pi platforms, I'd be more inclined to merge.

Do you have pointers on how I would get those metrics?

Print out how long it takes for a pipeline with and without the changes?

samfreund avatar Sep 22 '25 16:09 samfreund

@samfreund I tried using BenchmarkTest to compare the running times, but the results aren't very consistent. Here's the data from three runs from main:

====
5 second benchmark at resolution 1920x1440

Run 1
-----
Processing times - Min: 0.072ms (13921.178 FPS), Mean: 0.565ms (1771.477 FPS), Max: 5.307ms (188.43 FPS)
Latency times - Min: 0.297ms (3367.003 FPS), Mean: 1.701ms (588.025 FPS), Max: 6.221ms (160.746 FPS)

Run 2
-----
Processing times - Min: 0.079ms (12691.644 FPS), Mean: 0.5ms (2000.675 FPS), Max: 5.728ms (174.586 FPS)
Latency times - Min: 0.307ms (3257.329 FPS), Mean: 1.706ms (586.028 FPS), Max: 7.674ms (130.31 FPS)

Run 3
-----
Processing times - Min: 0.078ms (12861.405 FPS), Mean: 0.403ms (2483.919 FPS), Max: 0.857ms (1167.486 FPS)
Latency times - Min: 0.39ms (2564.103 FPS), Mean: 1.542ms (648.526 FPS), Max: 2.254ms (443.656 FPS)

====
5 second benchmark at resolution 640x480

Run 1
-----
Processing times - Min: 0.06ms (16725.485 FPS), Mean: 0.258ms (3877.954 FPS), Max: 1.923ms (520.032 FPS)
Latency times - Min: 0.111ms (9009.009 FPS), Mean: 0.51ms (1959.104 FPS), Max: 2.654ms (376.79 FPS)

Run 2
-----
Processing times - Min: 0.117ms (8550.078 FPS), Mean: 0.391ms (2554.983 FPS), Max: 0.804ms (1243.973 FPS)
Latency times - Min: 0.245ms (4081.633 FPS), Mean: 0.758ms (1319.805 FPS), Max: 1.736ms (576.037 FPS)

Run 3
-----
Processing times - Min: 0.12ms (8307.304 FPS), Mean: 0.412ms (2425.498 FPS), Max: 1.074ms (931.279 FPS)
Latency times - Min: 0.267ms (3745.318 FPS), Mean: 0.78ms (1282.428 FPS), Max: 1.741ms (574.383 FPS)

====
5 second benchmark at resolution 320x240

Run 1
-----
Processing times - Min: 0.06ms (16643.643 FPS), Mean: 0.295ms (3387.031 FPS), Max: 2.946ms (339.405 FPS)
Latency times - Min: 0.115ms (8695.652 FPS), Mean: 0.615ms (1626.788 FPS), Max: 5.685ms (175.901 FPS)

Run 2
-----
Processing times - Min: 0.077ms (13007.792 FPS), Mean: 0.357ms (2802.455 FPS), Max: 0.63ms (1587.511 FPS)
Latency times - Min: 0.133ms (7518.797 FPS), Mean: 0.646ms (1547.688 FPS), Max: 1.045ms (956.938 FPS)

Run 3
-----
Processing times - Min: 0.12ms (8365.261 FPS), Mean: 0.413ms (2423.391 FPS), Max: 1.669ms (599.176 FPS)
Latency times - Min: 0.221ms (4524.887 FPS), Mean: 0.745ms (1342.244 FPS), Max: 2.042ms (489.716 FPS)

====
5 second benchmark at resolution 1280x720

Run 1
-----
Processing times - Min: 0.059ms (17045.648 FPS), Mean: 0.222ms (4499.123 FPS), Max: 1.491ms (670.522 FPS)
Latency times - Min: 0.17ms (5882.353 FPS), Mean: 0.561ms (1783.061 FPS), Max: 1.99ms (502.513 FPS)

Run 2
-----
Processing times - Min: 0.058ms (17316.017 FPS), Mean: 0.25ms (4002.545 FPS), Max: 1.501ms (666.038 FPS)
Latency times - Min: 0.137ms (7299.27 FPS), Mean: 0.701ms (1425.813 FPS), Max: 2.269ms (440.723 FPS)

Run 3
-----
Processing times - Min: 0.058ms (17192.174 FPS), Mean: 0.338ms (2958.582 FPS), Max: 1.815ms (550.964 FPS)
Latency times - Min: 0.184ms (5434.783 FPS), Mean: 0.875ms (1142.803 FPS), Max: 2.425ms (412.371 FPS)

I would need to do a fair amount of runs to see what the distribution of baseline values are.

Does PhotonVision have better tools for measuring performance?

kcooney avatar Oct 13 '25 05:10 kcooney