Skip to content

Clean up file hash usage #10102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ private static class InputSummary {
private final ImmutableMap<String, String> bindingProperties;
private final long moduleLastModified;
private final long resourcesLastModified;
private final long filenameHash;
private final String filenameHash;

InputSummary(Map<String, String> bindingProperties, ModuleDef module) {
this.bindingProperties = ImmutableMap.copyOf(bindingProperties);
Expand All @@ -655,7 +655,7 @@ public boolean equals(Object obj) {
return bindingProperties.equals(other.bindingProperties) &&
moduleLastModified == other.moduleLastModified &&
resourcesLastModified == other.resourcesLastModified &&
filenameHash == other.filenameHash;
filenameHash.equals(other.filenameHash);
}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,19 @@
*/
package com.google.gwt.dev.codeserver;

import com.google.gwt.dev.util.Util;

import com.google.gwt.thirdparty.guava.common.hash.Hashing;
import junit.framework.TestCase;

import java.nio.charset.StandardCharsets;
import java.util.Locale;

/**
* Tests for {@link SourceHandler}
*/
public class SourceHandlerTest extends TestCase {

private static final String VALID_STRONG_NAME = Util.computeStrongName("foo-bar".getBytes());
private static final String VALID_STRONG_NAME = Hashing.murmur3_128()
.hashString("foo-bar", StandardCharsets.UTF_8).toString().toUpperCase(Locale.ROOT);

public void testIsSourceMapRequest() {
checkSourceMapRequest("/sourcemaps/myModule/");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@
import com.google.gwt.core.ext.TreeLogger;
import com.google.gwt.core.ext.UnableToCompleteException;
import com.google.gwt.dev.util.Util;
import com.google.gwt.thirdparty.guava.common.hash.Hashing;

import java.io.ByteArrayOutputStream;
import java.io.InputStream;
import java.util.Locale;

/**
* Provides basic functions common to all Linker implementations.
Expand Down Expand Up @@ -132,7 +134,8 @@ protected final SyntheticArtifact emitString(TreeLogger logger, String what,
protected final SyntheticArtifact emitWithStrongName(TreeLogger logger,
byte[] what, String prefix, String suffix)
throws UnableToCompleteException {
String strongName = prefix + Util.computeStrongName(what) + suffix;
String hash = Hashing.murmur3_128().hashBytes(what).toString().toUpperCase(Locale.ROOT);
String strongName = prefix + hash + suffix;
return emitBytes(logger, what, strongName);
}

Expand All @@ -152,7 +155,8 @@ protected final SyntheticArtifact emitWithStrongName(TreeLogger logger,
protected final SyntheticArtifact emitWithStrongName(TreeLogger logger,
byte[] what, String prefix, String suffix, long lastModified)
throws UnableToCompleteException {
String strongName = prefix + Util.computeStrongName(what) + suffix;
String hash = Hashing.murmur3_128().hashBytes(what).toString().toUpperCase(Locale.ROOT);
String strongName = prefix + hash + suffix;
return emitBytes(logger, what, strongName, lastModified);
}
}
34 changes: 19 additions & 15 deletions dev/core/src/com/google/gwt/dev/MinimalRebuildCacheManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,10 @@
import com.google.gwt.thirdparty.guava.common.annotations.VisibleForTesting;
import com.google.gwt.thirdparty.guava.common.cache.Cache;
import com.google.gwt.thirdparty.guava.common.cache.CacheBuilder;
import com.google.gwt.thirdparty.guava.common.io.Closeables;
import com.google.gwt.thirdparty.guava.common.hash.Hashing;
import com.google.gwt.thirdparty.guava.common.util.concurrent.Futures;
import com.google.gwt.thirdparty.guava.common.util.concurrent.MoreExecutors;
import com.google.gwt.util.tools.shared.Md5Utils;
import com.google.gwt.util.tools.shared.StringUtils;
import com.google.gwt.thirdparty.guava.common.io.Closeables;

import java.io.BufferedInputStream;
import java.io.BufferedOutputStream;
Expand All @@ -33,6 +32,7 @@
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.nio.charset.StandardCharsets;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.concurrent.Callable;
Expand Down Expand Up @@ -270,22 +270,26 @@ private String computeMinimalRebuildCacheName(String moduleName,
String currentWorkingDirectory = System.getProperty("user.dir");
String compilerVersionHash = CompilerVersion.getHash();
String permutationDescriptionString = permutationDescription.toString();
String optionsDescriptionString = " Options [";
StringBuilder optionsDescriptionString = new StringBuilder(" Options [");
String separator = "";
for (Map.Entry entry : options.entrySet()) {
optionsDescriptionString +=
String.format("%s%s = %s", separator, entry.getKey(), entry.getValue());
for (Map.Entry<String, String> entry : options.entrySet()) {
optionsDescriptionString.append(String.format("%s%s = %s", separator, entry.getKey(), entry.getValue()));
separator = ",";
}
optionsDescriptionString += "]";
optionsDescriptionString.append("]");

String consistentHash = StringUtils.toHexString(Md5Utils.getMd5Digest((
compilerVersionHash
+ moduleName
+ currentWorkingDirectory
+ permutationDescriptionString
+ optionsDescriptionString)
.getBytes()));
String consistentHash = Hashing.murmur3_128().newHasher()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Collisions from this are especially bad for the compiler, and should be revisited (so that collisions don't result in broken code).

// Compiler hash is constant length, no need to write length
.putString(compilerVersionHash, StandardCharsets.UTF_8)
.putInt(moduleName.length())
.putString(moduleName, StandardCharsets.UTF_8)
.putInt(currentWorkingDirectory.length())
.putString(currentWorkingDirectory, StandardCharsets.UTF_8)
// permutationDescriptionString has start/end delimiter
.putString(permutationDescriptionString, StandardCharsets.UTF_8)
// optionsDescriptionString has start/end delimiter
.putString(optionsDescriptionString, StandardCharsets.UTF_8)
.hash().toString();
return REBUILD_CACHE_PREFIX + "-" + consistentHash;
}

Expand Down
29 changes: 11 additions & 18 deletions dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,17 @@
import com.google.gwt.dev.util.log.speedtracer.CompilerEventType;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger.Event;
import com.google.gwt.thirdparty.guava.common.base.Charsets;
import com.google.gwt.thirdparty.guava.common.base.Predicates;
import com.google.gwt.thirdparty.guava.common.collect.ImmutableList;
import com.google.gwt.thirdparty.guava.common.collect.Iterators;
import com.google.gwt.thirdparty.guava.common.collect.Lists;
import com.google.gwt.thirdparty.guava.common.collect.Maps;
import com.google.gwt.thirdparty.guava.common.collect.Sets;
import com.google.gwt.thirdparty.guava.common.hash.Hasher;
import com.google.gwt.thirdparty.guava.common.hash.Hashing;

import java.io.File;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -455,31 +455,24 @@ public String getGwtXmlFilePath(String moduleName) {
* For example, consider a glob that matches fewer files than before because a file was
* deleted.
*/
public int getInputFilenameHash() {
List<String> filenames = new ArrayList<String>();

filenames.addAll(gwtXmlPathByModuleName.values());
public String getInputFilenameHash() {
List<String> filenames = new ArrayList<>(gwtXmlPathByModuleName.values());

for (Resource resource : getResourcesNewerThan(Integer.MIN_VALUE)) {
filenames.add(resource.getLocation());
}

// Take the first four bytes of the SHA-1 hash.
// Take the first eight bytes of the murmur3 hash.

Collections.sort(filenames);

MessageDigest digest;
try {
digest = MessageDigest.getInstance("SHA-1");
} catch (NoSuchAlgorithmException e) {
throw new RuntimeException("SHA-1 unavailable", e);
}
Hasher hasher = Hashing.murmur3_128().newHasher();
hasher.putInt(filenames.size());
for (String filename : filenames) {
digest.update(filename.getBytes(Charsets.UTF_8));
hasher.putInt(filename.length());
hasher.putString(filename, StandardCharsets.UTF_8);
}
byte[] bytes = digest.digest();

return (bytes[0] << 24) | (bytes[1] << 16) | (bytes[2] << 8) | bytes[3];
return hasher.hash().toString();
}

public Class<? extends Linker> getLinker(String name) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/
package com.google.gwt.dev.javac;

import com.google.gwt.dev.util.Util;
import com.google.gwt.thirdparty.guava.common.hash.Hashing;

import org.objectweb.asm.AnnotationVisitor;
import org.objectweb.asm.Attribute;
Expand All @@ -24,6 +24,7 @@
import org.objectweb.asm.FieldVisitor;
import org.objectweb.asm.Opcodes;

import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -60,7 +61,7 @@ public CompileDependencyVisitor() {
}

public String getSignature() {
return Util.computeStrongName(Util.getBytes(getRawString()));
return Hashing.murmur3_128().hashString(getRawString(), StandardCharsets.UTF_8).toString();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Collisions in this result in the compiler failing to generated code correctly in a variety of ways - this should be revisited to find a safer way to handle collisions.

}

@Override
Expand Down Expand Up @@ -203,7 +204,7 @@ private String getRawString() {
* methods in a class.
*
* @param byteCode byte code for class to analyze.
* @return a hex string representing an MD5 digest.
* @return a hex string representing a murmur3 hash.
*/
public static String getCompileDependencySignature(byte[] byteCode) {
CompileDependencyVisitor v = visitCompileDependenciesInBytecode(byteCode);
Expand Down
21 changes: 11 additions & 10 deletions dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import com.google.gwt.dev.js.ast.JsRootScope;
import com.google.gwt.dev.resource.Resource;
import com.google.gwt.dev.util.StringInterner;
import com.google.gwt.dev.util.Util;
import com.google.gwt.dev.util.log.speedtracer.CompilerEventType;
import com.google.gwt.dev.util.log.speedtracer.DevModeEventType;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger;
Expand All @@ -39,6 +38,10 @@
import com.google.gwt.thirdparty.guava.common.collect.Lists;
import com.google.gwt.thirdparty.guava.common.collect.Maps;
import com.google.gwt.thirdparty.guava.common.collect.Sets;
import com.google.gwt.thirdparty.guava.common.hash.Funnels;
import com.google.gwt.thirdparty.guava.common.hash.Hasher;
import com.google.gwt.thirdparty.guava.common.hash.Hashing;
import com.google.gwt.thirdparty.guava.common.io.ByteStreams;

import org.eclipse.jdt.core.compiler.CharOperation;
import org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration;
Expand All @@ -48,13 +51,13 @@
import org.eclipse.jdt.internal.compiler.lookup.Binding;
import org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
Expand Down Expand Up @@ -553,23 +556,21 @@ private boolean verifyContentId(TreeLogger logger, Resource resource, Compilatio
}

private ContentId getResourceContentId(Resource resource) {
ByteArrayOutputStream out = new ByteArrayOutputStream(1024);
try {
InputStream in = resource.openContents();
/**
try (InputStream in = resource.openContents()) {
/*
* In most cases openContents() will throw an exception, however in the case of a
* ZipFileResource it might return null causing an NPE in Util.copyNoClose(), see issue 4359.
*/
if (in == null) {
throw new RuntimeException("Unexpected error reading resource '" + resource + "'");
}
// TODO: deprecate com.google.gwt.dev.util.Util and use Guava.
Util.copy(in, out);
Hasher hasher = Hashing.murmur3_128().newHasher();
ByteStreams.copy(in, Funnels.asOutputStream(hasher));
String hash = hasher.hash().toString().toUpperCase(Locale.ROOT);
return new ContentId(Shared.getTypeName(resource), hash);
} catch (IOException e) {
throw new RuntimeException("Unexpected error reading resource '" + resource + "'", e);
}
byte[] content = out.toByteArray();
return new ContentId(Shared.getTypeName(resource), Util.computeStrongName(content));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import com.google.gwt.dev.jjs.ast.JDeclaredType;
import com.google.gwt.dev.resource.Resource;
import com.google.gwt.dev.util.Util;
import com.google.gwt.thirdparty.guava.common.hash.Hashing;
import com.google.gwt.thirdparty.guava.common.hash.HashingOutputStream;

import org.eclipse.jdt.core.compiler.CategorizedProblem;

Expand All @@ -26,6 +28,7 @@
import java.io.InputStream;
import java.util.Collection;
import java.util.List;
import java.util.Locale;

/**
* Builds a {@link CompilationUnit}.
Expand Down Expand Up @@ -135,22 +138,23 @@ protected String doGetSource() {
*/
lastModifed = resource.getLastModified();
ByteArrayOutputStream out = new ByteArrayOutputStream(1024);
HashingOutputStream hasher = new HashingOutputStream(Hashing.murmur3_128(), out);
try {
InputStream in = resource.openContents();
/**
/*
* In most cases openContents() will throw an exception, however in the case of a
* ZipFileResource it might return null causing an NPE in Util.copyNoClose(),
* see issue 4359.
*/
if (in == null) {
throw new RuntimeException("Unexpected error reading resource '" + resource + "'");
}
Util.copy(in, out);
Util.copy(in, hasher);
} catch (IOException e) {
throw new RuntimeException("Unexpected error reading resource '" + resource + "'", e);
}
byte[] content = out.toByteArray();
contentId = new ContentId(getTypeName(), Util.computeStrongName(content));
contentId = new ContentId(getTypeName(), hasher.hash().toString().toUpperCase(Locale.ROOT));
return Util.toString(content);
}

Expand Down
8 changes: 0 additions & 8 deletions dev/core/src/com/google/gwt/dev/javac/ContentId.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,7 @@
* A key that encapsulates one revision of the source code content for a type.
*/
class ContentId extends StringKey {

private String sourceTypeName;

public ContentId(String sourceTypeName, String strongHash) {
super(sourceTypeName + ':' + strongHash);
this.sourceTypeName = sourceTypeName;
}

public String getSourceTypeName() {
return sourceTypeName;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import com.google.gwt.dev.util.log.speedtracer.CompilerEventType;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger.Event;
import com.google.gwt.thirdparty.guava.common.hash.Hashing;
import com.google.gwt.thirdparty.guava.common.io.Files;

import java.io.ByteArrayOutputStream;
Expand All @@ -54,10 +55,12 @@
import java.io.OutputStream;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
Expand Down Expand Up @@ -149,8 +152,9 @@ public void abort() {
@Override
public void commit(TreeLogger logger) {
String source = sw.toString();
strongHash = Util.computeStrongName(Util.getBytes(source));
sourceToken = diskCache.writeString(source);
byte[] sourceBytes = source.getBytes(StandardCharsets.UTF_8);
strongHash = Hashing.murmur3_128().hashBytes(sourceBytes).toString().toUpperCase(Locale.ROOT);
sourceToken = diskCache.writeByteArray(sourceBytes);
Comment on lines +155 to +157
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Collisions from this are especially bad for the correct output, and should be revisited (so that collisions don't result in broken code).

sw = null;
creationTime = System.currentTimeMillis();
}
Expand Down
Loading