From c78e3877abf393522566cd24bfc8e27c133f581d Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Fri, 29 May 2020 22:04:49 +0200 Subject: [PATCH] [updater] more efficient use of collections; an attempt on parallelizing patch calculation GitOrigin-RevId: f09bda6a3253b9c9229ad9cbd45ce50bfebde10f --- .../com/intellij/updater/DiffCalculator.java | 12 ++-- updater/src/com/intellij/updater/Patch.java | 38 +++++++----- .../intellij/updater/PatchFileCreator.java | 60 ++++++++++++++++--- updater/src/com/intellij/updater/Utils.java | 7 +-- .../com/intellij/updater/PatchTestCase.java | 2 +- .../ZipAwarePatchApplyingRevertingTest.java | 11 ++-- 6 files changed, 93 insertions(+), 37 deletions(-) diff --git a/updater/src/com/intellij/updater/DiffCalculator.java b/updater/src/com/intellij/updater/DiffCalculator.java index 03c1f8e27a8e..7647acc2412a 100644 --- a/updater/src/com/intellij/updater/DiffCalculator.java +++ b/updater/src/com/intellij/updater/DiffCalculator.java @@ -1,4 +1,4 @@ -// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +// Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.intellij.updater; import java.io.File; @@ -6,13 +6,13 @@ import java.util.*; public class DiffCalculator { public static Result calculate(Map oldChecksums, Map newChecksums) { - return calculate(oldChecksums, newChecksums, Collections.emptyList(), Collections.emptyList(), false); + return calculate(oldChecksums, newChecksums, Collections.emptySet(), Collections.emptySet(), false); } public static Result calculate(Map oldChecksums, Map newChecksums, - List critical, - List optional, + Set critical, + Set optional, boolean lookForMoved) { Result result = new Result(); result.commonFiles = collect(oldChecksums, newChecksums, critical, true); @@ -76,7 +76,7 @@ public class DiffCalculator { return matches; } - private static String findBestCandidateForMove(List paths, String path, List optional) { + private static String findBestCandidateForMove(List paths, String path, Set optional) { if (paths == null) return null; boolean mandatory = !optional.contains(path); @@ -143,7 +143,7 @@ public class DiffCalculator { return result; } - private static Map collect(Map older, Map newer, List critical, boolean equal) { + private static Map collect(Map older, Map newer, Set critical, boolean equal) { Map result = new LinkedHashMap<>(); for (Map.Entry each : newer.entrySet()) { String file = each.getKey(); diff --git a/updater/src/com/intellij/updater/Patch.java b/updater/src/com/intellij/updater/Patch.java index d4da87488a26..3f3764e575d2 100644 --- a/updater/src/com/intellij/updater/Patch.java +++ b/updater/src/com/intellij/updater/Patch.java @@ -57,9 +57,17 @@ public class Patch { File olderDir = new File(spec.getOldFolder()); File newerDir = new File(spec.getNewFolder()); - Map oldChecksums = digestFiles(olderDir, spec.getIgnoredFiles(), isNormalized()); - Map newChecksums = digestFiles(newerDir, spec.getIgnoredFiles(), false); - DiffCalculator.Result diff = DiffCalculator.calculate(oldChecksums, newChecksums, spec.getCriticalFiles(), spec.getOptionalFiles(), true); + + Set ignored = new HashSet<>(spec.getIgnoredFiles()); + Set critical = new HashSet<>(spec.getCriticalFiles()); + Set optional = new HashSet<>(spec.getOptionalFiles()); + + Map oldChecksums = digestFiles(olderDir, ignored, isNormalized()); + Map newChecksums = digestFiles(newerDir, ignored, false); + DiffCalculator.Result diff = DiffCalculator.calculate(oldChecksums, newChecksums, critical, optional, true); + + Runner.logger().info("Preparing actions..."); + ui.startProcess("Preparing actions..."); List tempActions = new ArrayList<>(); @@ -89,16 +97,13 @@ public class Patch { } } - Runner.logger().info("Preparing actions..."); - ui.startProcess("Preparing actions..."); - List actions = new ArrayList<>(); for (PatchAction action : tempActions) { Runner.logger().info(action.getPath()); if (action.calculate(olderDir, newerDir)) { actions.add(action); - action.setCritical(spec.getCriticalFiles().contains(action.getPath())); - action.setOptional(spec.getOptionalFiles().contains(action.getPath())); + action.setCritical(critical.contains(action.getPath())); + action.setOptional(optional.contains(action.getPath())); } } return actions; @@ -388,14 +393,19 @@ public class Patch { } } - public Map digestFiles(File dir, List ignoredFiles, boolean normalize) throws IOException { + public Map digestFiles(File dir, Set ignoredFiles, boolean normalize) throws IOException { Map result = new LinkedHashMap<>(); - LinkedHashSet paths = Utils.collectRelativePaths(dir.toPath()); - for (String each : paths) { - if (!ignoredFiles.contains(each)) { - result.put(each, digestFile(new File(dir, each), normalize)); + Utils.collectRelativePaths(dir.toPath()).parallelStream().forEachOrdered(path -> { + if (!ignoredFiles.contains(path)) { + try { + long hash = digestFile(new File(dir, path), normalize); + synchronized (result) { + result.put(path, hash); + } + } + catch (IOException e) { throw new UncheckedIOException(e); } } - } + }); return result; } diff --git a/updater/src/com/intellij/updater/PatchFileCreator.java b/updater/src/com/intellij/updater/PatchFileCreator.java index d466f358a74e..679e6d8de215 100644 --- a/updater/src/com/intellij/updater/PatchFileCreator.java +++ b/updater/src/com/intellij/updater/PatchFileCreator.java @@ -1,14 +1,18 @@ -// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +// Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.intellij.updater; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.List; import java.util.Map; +import java.util.concurrent.*; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; +import java.util.zip.ZipInputStream; import java.util.zip.ZipOutputStream; public class PatchFileCreator { @@ -20,22 +24,64 @@ public class PatchFileCreator { Patch patchInfo = new Patch(spec, ui); + Runner.logger().info("Packing entries..."); + ui.startProcess("Packing entries..."); + + List actions = patchInfo.getActions(); + File olderDir = new File(spec.getOldFolder()); + File newerDir = new File(spec.getNewFolder()); + ExecutorService executor = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors() - 1); + Map> tasks = new ConcurrentHashMap<>(); + + for (int i = 0; i < actions.size(); i++) { + PatchAction action = actions.get(i); + if (action instanceof UpdateAction && !action.isCritical()) { + int _i = i; + tasks.put(action, executor.submit(() -> { + Path temp = Utils.getTempFile("diff_" + _i).toPath(); + try (ZipOutputStream out = new ZipOutputStream(Files.newOutputStream(temp))) { + out.setLevel(0); + action.buildPatchFile(olderDir, newerDir, out); + } + return temp; + })); + } + } + try (ZipOutputStream out = new ZipOutputStream(new FileOutputStream(patchFile))) { out.setLevel(9); + Runner.logger().info("Packing " + PATCH_INFO_FILE_NAME); out.putNextEntry(new ZipEntry(PATCH_INFO_FILE_NAME)); patchInfo.write(out); out.closeEntry(); - File olderDir = new File(spec.getOldFolder()); - File newerDir = new File(spec.getNewFolder()); - List actions = patchInfo.getActions(); - for (PatchAction each : actions) { - Runner.logger().info("Packing " + each.getPath()); - each.buildPatchFile(olderDir, newerDir, out); + for (PatchAction action : actions) { + Runner.logger().info("Packing " + action.getPath()); + Future task = tasks.get(action); + if (task == null) { + action.buildPatchFile(olderDir, newerDir, out); + } + else { + try { + Path temp = task.get(); + try (ZipInputStream in = new ZipInputStream(Files.newInputStream(temp))) { + ZipEntry entry; + while ((entry = in.getNextEntry()) != null) { + out.putNextEntry(new ZipEntry(entry.getName())); + Utils.copyStream(in, out); + out.closeEntry(); + } + } + } + catch (InterruptedException e) { throw new IOException(e); } + catch (ExecutionException e) { throw ((IOException)e.getCause()); } + } } } + executor.shutdown(); + return patchInfo; } diff --git a/updater/src/com/intellij/updater/Utils.java b/updater/src/com/intellij/updater/Utils.java index a8f8c94ef69a..a13bc678503b 100644 --- a/updater/src/com/intellij/updater/Utils.java +++ b/updater/src/com/intellij/updater/Utils.java @@ -17,7 +17,6 @@ public class Utils { private static final long REQUIRED_FREE_SPACE = 2_000_000_000L; private static final int BUFFER_SIZE = 8192; // to minimize native memory allocations for I/O operations - private static final byte[] BUFFER = new byte[BUFFER_SIZE]; private static File myTempDir; @@ -220,10 +219,11 @@ public class Utils { } public static void copyStream(InputStream from, OutputStream to) throws IOException { + byte[] buffer = new byte[BUFFER_SIZE]; while (true) { - int read = from.read(BUFFER); + int read = from.read(buffer); if (read < 0) break; - to.write(BUFFER, 0, read); + to.write(buffer, 0, read); } } @@ -337,7 +337,6 @@ public class Utils { @Override @SuppressWarnings("NonPrivateFieldAccessedInSynchronizedContext") public synchronized void writeTo(OutputStream out) throws IOException { - //noinspection UnnecessarilyQualifiedStaticUsage Utils.writeBytes(buf, count, out); } } diff --git a/updater/testSrc/com/intellij/updater/PatchTestCase.java b/updater/testSrc/com/intellij/updater/PatchTestCase.java index 8305a41bbd9a..ccd91a54bcbd 100644 --- a/updater/testSrc/com/intellij/updater/PatchTestCase.java +++ b/updater/testSrc/com/intellij/updater/PatchTestCase.java @@ -65,7 +65,7 @@ public abstract class PatchTestCase extends UpdaterTestCase { } protected static Map digest(Patch patch, File dir) throws IOException { - return new TreeMap<>(patch.digestFiles(dir, Collections.emptyList(), false)); + return new TreeMap<>(patch.digestFiles(dir, Collections.emptySet(), false)); } protected static List sortActions(List actions) { diff --git a/updater/testSrc/com/intellij/updater/ZipAwarePatchApplyingRevertingTest.java b/updater/testSrc/com/intellij/updater/ZipAwarePatchApplyingRevertingTest.java index 32995c059a82..c36e18e51839 100644 --- a/updater/testSrc/com/intellij/updater/ZipAwarePatchApplyingRevertingTest.java +++ b/updater/testSrc/com/intellij/updater/ZipAwarePatchApplyingRevertingTest.java @@ -20,8 +20,9 @@ import org.junit.Test; import java.io.File; import java.io.IOException; +import java.io.UncheckedIOException; -import static org.junit.Assert.assertEquals; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.fail; public class ZipAwarePatchApplyingRevertingTest extends PatchApplyingRevertingTest { @@ -38,8 +39,8 @@ public class ZipAwarePatchApplyingRevertingTest extends PatchApplyingRevertingTe createPatch(); fail("Should have failed to create a patch against empty .jar"); } - catch (IOException e) { - assertEquals("Corrupted file: " + targetJar, e.getMessage()); + catch (IOException | UncheckedIOException e) { + assertThat(e.getMessage()).endsWith("Corrupted file: " + targetJar); } finally { FileUtil.delete(targetJar); @@ -63,8 +64,8 @@ public class ZipAwarePatchApplyingRevertingTest extends PatchApplyingRevertingTe createPatch(); fail("Should have failed to create a patch from empty .jar"); } - catch (IOException e) { - assertEquals("Corrupted file: " + sourceJar, e.getMessage()); + catch (IOException | UncheckedIOException e) { + assertThat(e.getMessage()).endsWith("Corrupted file: " + sourceJar); } finally { FileUtil.delete(targetJar);