checkstyle / checkstyle/checkstyle
Performance: Avoid duplicate regex match in SuppressFilterElement.isFileMatch
- Dominant language
- Java
- Stars
- 9.6k
- Forks
- 4.2k
- Avg merge
- 22h 23m
- Merged PRs (30d)
- 232
Description
I have read check documentation: https://checkstyle.org/filters/suppressionfilter.html
I have downloaded the latest cli from: https://checkstyle.org/cmdline.html#Download_and_Run
I have executed the cli and showed it below, as cli describes the problem better than 1,000 words
**How it works Now:**
```bash
/var/tmp $ javac Test.java
/var/tmp $ cat config.xml
/var/tmp $ tail -n 10 large-suppressions.xml
/var/tmp $ cat Test.java
public class Test {
public void foo() {}
}
/var/tmp $ RUN_LOCALE="-Duser.language=en -Duser.country=US"
/var/tmp $ java $RUN_LOCALE -jar checkstyle-10.21.4-all.jar -c config.xml Test.java
Starting audit...
Audit done.
```
**Is your feature request related to a problem? Please describe.**
We maintain a codebase with 10,000+ suppression entries generated for legacy files:
```xml
```
In a real project, we run Checkstyle through Maven across tens of thousands of files. When profiling Checkstyle
runs (inside the Maven Checkstyle Plugin), over 10 seconds was spent inside
[`SuppressFilterElement.isFileMatch`](https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressFilterElement.java#L168-L175).
In `SuppressFilterElement.isFileMatch(String fileName)`:
```java
private boolean isFileMatch(String fileName) {
boolean match = fileRegexp.matcher(fileName).find();
if (!match) {
final String slashesFileName = fileName.replace('\\', '/');
match = fileRegexp.matcher(slashesFileName).find();
}
return match;
}
```
When a file path has no backslashes (`\`) (which is typical in our environments) `fileName.replace('\\', '/')`
returns the same string instance. Calling `fileRegexp.matcher(slashesFileName).find()`
again executes a duplicate regex.
I am proposing *a simple fix* that cuts the number of regex matches in half.
**Describe the solution you'd like**
Add a check (`slashesFileName != fileName`) to skip the second regex match when no substitution occurred:
```java
private boolean isFileMatch(String fileName) {
boolean match = fileRegexp.matcher(fileName).find();
if (!match) {
final String slashesFileName = fileName.replace('\\', '/');
if (slashesFileName != fileName) {
match = fileRegexp.matcher(slashesFileName).find();
}
}
return match;
}
```
Note that we don't need equals() as String.replace() returns the same instance when there's nothing to replace.
Contributor guide
Research direction
Start in src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressFilterElement.java and inspect isFileMatch(String fileName), using the supplied suppression configuration and CLI example to understand the current behavior. Ensure paths without backslashes do not trigger a duplicate regex match while slash normalization still works, then run the relevant filter tests if available.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- performance
- Issue type
- Refactor
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100