Skip to content

Commit

Permalink
[perf] Rewrite the formatting logic so it only reads file once and wr…
Browse files Browse the repository at this point in the history 
…ites once

Previously it would read the file up to 3 times and write it up to 2 times.  With this change, it will hold the file content it is working with skipping need to read/write extra times.
  • Loading branch information
@hazendaz
hazendaz committed Jun 27, 2020
1 parent 68a725f commit 51b4e7c
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 41 deletions.
23 changes: 7 additions & 16 deletions src/main/java/net/revelc/code/formatter/AbstractCacheableFormatter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.util.Objects;

import org.apache.maven.plugin.logging.Log;
import org.codehaus.plexus.util.FileUtils;
import org.eclipse.jface.text.BadLocationException;
import org.eclipse.text.edits.MalformedTreeException;

Expand All @@ -40,42 +39,34 @@ protected void initCfg(ConfigurationSource cfg) {
this.encoding = cfg.getEncoding();
}

public Result formatFile(File file, LineEnding ending, boolean dryRun) {
public String formatFile(File file, String originalCode, LineEnding ending) {
try {
this.log.debug("Processing file: " + file + " with line ending: " + ending);
String code = FileUtils.fileRead(file, this.encoding.name());
LineEnding formatterLineEnding = ending;
// if the line ending is set as KEEP we have to determine the current line ending of the file
// and let the formatter use this one. Otherwise it would likely fall back to current system line separator
if (formatterLineEnding == LineEnding.KEEP) {
formatterLineEnding = LineEnding.determineLineEnding(code);
formatterLineEnding = LineEnding.determineLineEnding(originalCode);
this.log.debug("Determined line ending: " + formatterLineEnding + " to keep for file: " + file);
}
String formattedCode = doFormat(code, formatterLineEnding);
String formattedCode = doFormat(originalCode, formatterLineEnding);

if (formattedCode == null) {
this.log.debug("Nothing formatted. Try to fix line endings.");
formattedCode = fixLineEnding(code, ending);
formattedCode = fixLineEnding(originalCode, ending);
}

if (formattedCode == null) {
this.log.debug("Equal code. Not writing result to file.");
return Result.SKIPPED;
return originalCode;
}

this.log.debug("Line endings fixed");

if (!dryRun) {
FileUtils.fileWrite(file, this.encoding.name(), formattedCode);
}

// readme: Uncomment this when having build issues with hashCodes when nothing
// changed. The issue is likely copyright dating issues.
// this.log.debug("formatted code: " + formattedCode);
return Result.SUCCESS;
return formattedCode;
} catch (IOException | MalformedTreeException | BadLocationException e) {
this.log.warn(e);
return Result.FAIL;
return null;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/net/revelc/code/formatter/Formatter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public interface Formatter {
/**
* Format individual file.
*/
abstract Result formatFile(File file, LineEnding ending, boolean dryRun);
abstract String formatFile(File file, String originalCode, LineEnding ending);

/**
* return true if this formatter have been initialized
Expand Down
60 changes: 38 additions & 22 deletions src/main/java/net/revelc/code/formatter/FormatterMojo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import org.eclipse.text.edits.MalformedTreeException;
import org.xml.sax.SAXException;

import com.google.common.base.Strings;
import com.google.common.hash.Hashing;

import net.revelc.code.formatter.css.CssFormatter;
Expand Down Expand Up @@ -506,8 +507,8 @@ protected void doFormatFile(File file, ResultCollector rc, Properties hashCache,
throws IOException, BadLocationException, MojoFailureException, MojoExecutionException {
Log log = getLog();
log.debug("Processing file: " + file);
String code = readFileAsString(file);
String originalHash = sha512hash(code);
String originalCode = readFileAsString(file);
String originalHash = sha512hash(originalCode);

String canonicalPath = file.getCanonicalPath();
String path = canonicalPath.substring(basedirPath.length());
Expand All @@ -518,87 +519,102 @@ protected void doFormatFile(File file, ResultCollector rc, Properties hashCache,
return;
}

Result result;
Result result = null;
String formattedCode = null;
if (file.getName().endsWith(".java") && javaFormatter.isInitialized()) {
if (skipJavaFormatting) {
getLog().info("Java formatting is skipped");
result = Result.SKIPPED;
} else {
result = this.javaFormatter.formatFile(file, this.lineEnding, dryRun);
formattedCode = this.javaFormatter.formatFile(file, originalCode, this.lineEnding);
}
} else if (file.getName().endsWith(".js") && jsFormatter.isInitialized()) {
if (skipJsFormatting) {
getLog().info("Javascript formatting is skipped");
result = Result.SKIPPED;
} else {
result = this.jsFormatter.formatFile(file, this.lineEnding, dryRun);
formattedCode = this.jsFormatter.formatFile(file, originalCode, this.lineEnding);
}
} else if (file.getName().endsWith(".html") && htmlFormatter.isInitialized()) {
if (skipHtmlFormatting) {
getLog().info("Html formatting is skipped");
result = Result.SKIPPED;
} else {
result = this.htmlFormatter.formatFile(file, this.lineEnding, dryRun);
formattedCode = this.htmlFormatter.formatFile(file, originalCode, this.lineEnding);
}
} else if (file.getName().endsWith(".xml") && xmlFormatter.isInitialized()) {
if (skipXmlFormatting) {
getLog().info("Xml formatting is skipped");
result = Result.SKIPPED;
} else {
result = this.xmlFormatter.formatFile(file, this.lineEnding, dryRun);
formattedCode = this.xmlFormatter.formatFile(file, originalCode, this.lineEnding);
}
} else if (file.getName().endsWith(".json") && jsonFormatter.isInitialized()) {
if (skipJsonFormatting) {
getLog().info("json formatting is skipped");
result = Result.SKIPPED;
} else {
result = this.jsonFormatter.formatFile(file, this.lineEnding, dryRun);
formattedCode = this.jsonFormatter.formatFile(file, originalCode, this.lineEnding);
}
} else if (file.getName().endsWith(".css") && cssFormatter.isInitialized()) {
if (skipCssFormatting) {
getLog().info("css formatting is skipped");
result = Result.SKIPPED;
} else {
result = this.cssFormatter.formatFile(file, this.lineEnding, dryRun);
formattedCode = this.cssFormatter.formatFile(file, originalCode, this.lineEnding);
}
} else {
log.debug("No formatter found or initialization failed for file " + file.getName());
result = Result.SKIPPED;
}

switch (result) {
case SKIPPED:
// If not skipped, check formatting result
if (!Result.SKIPPED.equals(result)) {
if (formattedCode == null) {
result = Result.FAIL;
} else if (originalCode.equals(formattedCode)) {
result = Result.SKIPPED;
} else {
result = Result.SUCCESS;
}
}

// Process the result type
if (Result.SKIPPED.equals(result)) {
rc.skippedCount++;
break;
case SUCCESS:
} else if (Result.SUCCESS.equals(result)) {
rc.successCount++;
break;
case FAIL:
} else if (Result.FAIL.equals(result)) {
rc.failCount++;
return;
default:
break;
}

// Write the cache
String formattedCode = readFileAsString(file);
String formattedHash = sha512hash(formattedCode);
String formattedHash;
if (Result.SKIPPED.equals(result)) {
formattedHash = originalHash;
} else {
formattedHash = sha512hash(Strings.nullToEmpty(formattedCode));
}
hashCache.setProperty(path, formattedHash);

// If we had determined to skip write, do so now after cache was written
if (result.equals(Result.SKIPPED)) {
if (Result.SKIPPED.equals(result)) {
log.debug("File is already formatted. Writing to cache only.");
return;
}

// As a safety check, if our hash matches, skip the write of file
// As a safety check, if our hash matches, skip the write of file (should never occur)
if (originalHash.equals(formattedHash)) {
rc.skippedCount++;
log.debug("Equal hash code. Not writing result to file.");
return;
}

// Now write the file
writeStringToFile(formattedCode, file);
if (!dryRun) {
writeStringToFile(formattedCode, file);
}
}

/**
Expand Down
18 changes: 16 additions & 2 deletions src/test/java/net/revelc/code/formatter/AbstractFormatterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
package net.revelc.code.formatter;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.File;
Expand All @@ -25,6 +27,7 @@

import org.apache.maven.plugin.logging.Log;
import org.apache.maven.plugin.logging.SystemStreamLog;
import org.codehaus.plexus.util.FileUtils;
import org.junit.jupiter.api.BeforeAll;

import com.google.common.hash.Hashing;
Expand Down Expand Up @@ -85,13 +88,24 @@ protected void doTestFormat(Formatter formatter, String fileUnderTest, String ex
protected void doTestFormat(Map<String, String> options, Formatter formatter, String fileUnderTest,
String expectedSha512, LineEnding lineEnding) throws IOException {

// Set original file and file to use for test
File originalSourceFile = new File("src/test/resources/", fileUnderTest);
File sourceFile = new File(TEST_OUTPUT_DIR, fileUnderTest);

// Copy file to new location
Files.copy(originalSourceFile, sourceFile);

// Read file to be formatted
String originalCode = FileUtils.fileRead(sourceFile, StandardCharsets.UTF_8.name());

// Format the file and make sure formatting worked
formatter.init(options, new TestConfigurationSource(TEST_OUTPUT_DIR));
Result result = formatter.formatFile(sourceFile, lineEnding, false);
assertEquals(Result.SUCCESS, result);
String formattedCode = formatter.formatFile(sourceFile, originalCode, lineEnding);
assertNotNull(formattedCode);
assertNotEquals(originalCode, formattedCode);

// Write the file we formatte4d
FileUtils.fileWrite(sourceFile, StandardCharsets.UTF_8.name(), formattedCode);

// We are hashing this as set in stone in case for some reason our source file changes unexpectedly.
byte[] sha512 = Files.asByteSource(sourceFile).hash(Hashing.sha512()).asBytes();
Expand Down

0 comments on commit 51b4e7c

Please sign in to comment.