michael-o commented on pull request #28: URL: https://github.com/apache/maven-shared-utils/pull/28#issuecomment-634305327
> > > > Does it actually make sense to introduce a threshold? E.g., > 1 MB? > > You're suggesting that we apply the new logic only to input >1MB and for smaller files we presumably process them in memory and compare that to the existing written file, right? > > In general I dislike maintaining multiple equivalent codepaths and switching between them conditionally as typically one is used far more often and the others languish in bit rot, becoming a maintenance burden. Sometimes this approach is necessary to gain performance in different circumstances, but is that clearly the case here? > > I'm not sure it is. For a start I don't remember what rounding errors might have been involved in the 1 vs 8 millisecond comparison from the PR description, but more importantly that's comparing broken to fixed behaviour - the 1 millisecond figure timing is for the existing "unconditionally overwrite when filtering", and we don't yet have comparable numbers for "process them in memory and compare that to the existing written file". > > If an additional 7 milliseconds per filtered resource is enough to cause panic then I guess I can look into a thresholded approach but my inclination is to avoid that effort until it proves necessary. Well reasoned, let's leave it as-is. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
