kvr000 commented on PR #378: URL: https://github.com/apache/commons-compress/pull/378#issuecomment-4950639044
> In commit [a8cb750](https://github.com/apache/commons-compress/commit/a8cb750e5bdd0fe136d5d9cab38781f9447b2dd3), I introduced `org.apache.commons.compress.archivers.zip.ZipFileNameMapBenchmark`. > > However, the benchmark indicates a 30% performance degradation in one method. This raises concerns about whether the change is justified. Could you help me evaluate whether this trade-off is acceptable, or if I'm overlooking something? It depends on typical usage. The benchmark opens the file only once and then repeatedly requests the zip entry. As getEntries() now dynamically wrap the list with Collections.singletonList(), it adds to the time. However, is it the typical scenario that user opens the file once and then asks repeatedly for the same entry? Unlikely - maybe in jar files but I guess those are rather cached. A compromise may be to to use the original code and wrap it into Collections.singletonList() if there is single entry only - this will add one object wrapper only, improve the space consumption, improve population phase speed and keep the lookup at the same speed (rather slightly faster as iterating over hardcoded singleton list is more trivial). Alternatively, keeping this code and introduce getOnlyEntry() (crashing if there are more) and getFirstEntry() (return first if there are more) - which would be the typical usages anyway. Then the new code will be better by avoiding the singletonList() wrapper for all entries in memory and avoiding unnecessary iteration. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
