Bug 56801

Summary: Avoid duplicated String to CharArray conversion in the loop of Matcher#matchName
Product: Tomcat 7 Reporter: Sheldon Shao <xshao>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: trunk   
Target Milestone: ---   
Hardware: All   
OS: All   
Attachments: Patch for Matcher

Description Sheldon Shao 2014-08-01 02:31:24 UTC
Created attachment 31865 [details]
Patch for Matcher

The loop in matchName,

    public static boolean matchName(Set<String> patternSet, String fileName) {
        for (String pattern: patternSet) {
            if (match(pattern, fileName, true)) {
                return true;
            }
        }
        return false;
    }


Optimized,

    public static boolean matchName(Set<String> patternSet, String fileName) {
        char[] charArray = fileName.toCharArray();
        for (String pattern: patternSet) {
            if (match(pattern, charArray, true)) {
                return true;
            }
        }
        return false;
    }
Comment 1 Christopher Schultz 2014-08-01 22:13:31 UTC
I think the real question is whether or not the String.toCharArray even belongs here. Back in the dark days, only fools used String.charAt and instead used String.toCharArray followed by direct index access. I haven't done any performance measurements, but String.toCharArray may be more trouble than it's worth these days.
Comment 2 Mark Thomas 2014-08-05 07:39:34 UTC
Based on the quality of patches the OP has provided in the past, I'm confident that a) the proposed change will be faster and b) there is unlikely to be a better way.

I've applied a variation of this patch to 8.0.x for 8.0.11 onwards.
Comment 3 Konstantin Kolinko 2014-08-12 13:54:31 UTC
(In reply to Christopher Schultz from comment #1)
> I think the real question is whether or not the String.toCharArray even
> belongs here.

Agreed. I changed visibility of the new method to be private, so that we were able to review the API later.  There is also a question that patterns are also repeatedly converted to arrays.

I backported this to Tomcat 7 in r1617475 and it will be in 7.0.56 onwards.