Skip to content

Commit

Permalink
Merge branch '__rultor'
Browse files Browse the repository at this point in the history
  • Loading branch information
rultor committed Jan 26, 2024
2 parents 2d5a46e + e2ed9e7 commit 99ed421
Show file tree
Hide file tree
Showing 7 changed files with 163 additions and 111 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -144,15 +144,18 @@ private Path make(final XML xml, final Path file) throws IOException {
* @param tojo Tojo
* @param common Optimization
* @return Optimization for specific Tojo
* @throws Exception If fails
*/
private Optimization optimization(final ForeignTojo tojo, final Optimization common) {
private Optimization optimization(final ForeignTojo tojo, final Optimization common)
throws Exception {
final Optimization res;
if (tojo.hasHash()) {
res = new OptCached(
common,
this.paths.get(
OptimizationFolder.CACHE.key()
).resolve(this.dirs.get(OptimizationFolder.CACHE.key())).resolve(tojo.hash())
).resolve(this.dirs.get(OptimizationFolder.CACHE.key())).resolve(tojo.hash()),
this.source.apply(tojo)
);
} else {
res = common;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.BasicFileAttributes;
import java.time.ZonedDateTime;
import java.time.temporal.ChronoUnit;
import java.util.Optional;
import org.eolang.maven.AssembleMojo;
import org.eolang.maven.Place;
import org.eolang.maven.footprint.FtDefault;
Expand All @@ -41,19 +37,22 @@
* Returns already optimized XML if it's found in the cache.
*
* @since 0.28.11
* @todo #2746:30min Fix caching mechanism in {@link OptCached}. Current
* The last modified time of the files between stages may be different,
* so it is not correct to do an equality comparison ({@code .equals(...)}).
* The last modification time of the file at the current stage
* must be less than or equal to the last modification time of file in cache at the next stage.
* The following tests show that fetching from the cache doesn't work correctly:
* - {@link OptCachedTest#returnsFromCacheCorrectProgram(Path path)},
* - {@link OptCachedTest#returnsFromCacheButTimesSaveAndExecuteDifferent(Path path)}.
* @todo #2746:30min Unify caching mechanism on stages: parse, optimize, pull and so on.
* Current implementations of caching on parsing stage and optimize stages work differently.
* In ParseMojo we have condition {@code if (tojo.hasHash()) }, in OptimizeMojo or ShakeMojo we
* In ParseMojo we have condition {@code if (tojo.hasHash())}, in OptimizeMojo or ShakeMojo we
* compare creation time of files.
* Don't forget to enable the tests.
* @todo #2790:30min Refactor OptCache Class
* Currently, we have some concerns about the implementation of OptCache.
* It appears that the code is a bit too complicated.
* The OptCache class takes XMIR as an argument in OptCache#apply
* and the path to the same XMIR in the constructor, which seems odd.
* We need to consider how to refactor this class.
* Furthermore, the current implementation of OptCache
* and OptimizationTask has a similar logic of returning either from the cache
* or applying a default optimization.
* For more information, please refer to this discussion:
* <a href=“https://github.com/objectionary/eo/pull/2808#discussion_r1464941944”>issue</a>.
*/
public final class OptCached implements Optimization {

Expand All @@ -63,73 +62,85 @@ public final class OptCached implements Optimization {
private final Optimization delegate;

/**
* Cache folder.
* Absolute path of cache folder.
*/
private final Path folder;

/**
* Absolute path of program xml.
*/
private final Path path;

/**
* The main constructor.
*
* @param delegate Real optimization.
* @param folder Cache folder.
* @param path XML file path.
*/
public OptCached(
final Optimization delegate,
final Path folder
final Path folder,
final Path path
) {
this.delegate = delegate;
this.folder = folder;
this.path = path;
}

@Override
public XML apply(final XML xml) {
final String name = xml.xpath("/program/@name").get(0);
try {
final XML optimized;
if (this.contains(xml)) {
optimized = new XMLDocument(this.cached(xml));
if (this.contains(name)) {
optimized = new XMLDocument(this.cached(name));
} else {
optimized = this.delegate.apply(xml);
new FtDefault(this.folder).save(
xml.xpath("/program/@name").get(0),
name,
AssembleMojo.IR_EXTENSION,
optimized::toString
);
}
return optimized;
} catch (final IOException ex) {
throw new IllegalStateException(String.format("Can't optimize '%s'", xml), ex);
throw new IllegalStateException(
String.format("Can't optimize '%s'", xml),
ex
);
}
}

/**
* Returns the path to the cached program.
* Pay attention that the path is not checked for existence.
* @param xml Eo program.
* @param name Name XML program.
* @return Path to the cached program.
* @throws IOException If fails.
*/
private Path cached(final XML xml) {
return new Place(xml.xpath("/program/@name").get(0))
private Path cached(final String name) throws IOException {
return new Place(name)
.make(this.folder, AssembleMojo.IR_EXTENSION);
}

/**
* Checks if the cache contains the program.
* @param xml Eo program.
* @param name Name XML program.
* @return True if the cache contains the program.
* @throws IOException If fails.
*/
private boolean contains(final XML xml) throws IOException {
final Path path = this.cached(xml);
final Optional<String> time = xml.xpath("/program/@time").stream().findFirst();
private boolean contains(final String name) throws IOException {
final Path cache = this.cached(name);
final boolean res;
if (Files.exists(path) && time.isPresent()) {
res = Files.readAttributes(path, BasicFileAttributes.class)
.creationTime()
if (Files.exists(cache)) {
res = !Files
.getLastModifiedTime(cache)
.toInstant()
.truncatedTo(ChronoUnit.MINUTES)
.equals(
ZonedDateTime.parse(time.get()).toInstant().truncatedTo(ChronoUnit.MINUTES)
);
.isBefore(
Files.getLastModifiedTime(this.path)
.toInstant()
);
} else {
res = false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public OptSpy(final Path target) {

/**
* The main constructor.
* @param trn Optimizations train
* @param trn Optimizations train.
* @param target Where to track optimization steps.
*/
public OptSpy(final Train<Shift> trn, final Path target) {
Expand All @@ -67,8 +67,8 @@ public OptSpy(final Train<Shift> trn, final Path target) {

@Override
public XML apply(final XML xml) {
final Place place = new Place(xml.xpath("/program/@name").get(0));
final Path dir = place.make(this.target, "");
final Path dir = new Place(xml.xpath("/program/@name").get(0))
.make(this.target, "");
Logger.debug(
this, "Optimization steps will be tracked to %s",
new Rel(dir)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@

import com.jcabi.xml.XMLDocument;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.FileTime;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
Expand All @@ -43,7 +45,6 @@
import org.hamcrest.io.FileMatchers;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
Expand Down Expand Up @@ -118,7 +119,6 @@ void optimizesIfExpired(@TempDir final Path temp) throws Exception {
* then enable the test.
* Also, see this <a href="https://github.com/objectionary/eo/issues/2727">issue</a>.
*/
@Disabled
@Test
void getsAlreadyOptimizedResultsFromCache(@TempDir final Path temp) throws Exception {
final TextOf cached = new TextOf(
Expand All @@ -132,6 +132,15 @@ void getsAlreadyOptimizedResultsFromCache(@TempDir final Path temp) throws Excep
.resolve(hash)
.resolve("foo/x/main.xmir")
);
Files.setLastModifiedTime(
cache.resolve(
Paths
.get(OptimizeMojo.OPTIMIZED)
.resolve(hash)
.resolve("foo/x/main.xmir")
),
FileTime.fromMillis(System.currentTimeMillis() + 50_000)
);
new FakeMaven(temp)
.withHelloWorld()
.with("cache", cache)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@

import com.jcabi.xml.XMLDocument;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.FileTime;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import org.cactoos.io.ResourceOf;
Expand All @@ -37,7 +39,6 @@
import org.hamcrest.io.FileMatchers;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

Expand Down Expand Up @@ -82,7 +83,6 @@ void shakesSuccessfully(@TempDir final Path temp) throws IOException {
);
}

@Disabled
@Test
void getsAlreadyShakenResultsFromCache(@TempDir final Path temp) throws Exception {
final TextOf cached = new TextOf(
Expand All @@ -96,6 +96,10 @@ void getsAlreadyShakenResultsFromCache(@TempDir final Path temp) throws Exceptio
.resolve(hash)
.resolve("foo/x/main.xmir")
);
Files.setLastModifiedTime(
cache.resolve(Paths.get(ShakeMojo.SHAKEN).resolve(hash).resolve("foo/x/main.xmir")),
FileTime.fromMillis(System.currentTimeMillis() + 50_000)
);
new FakeMaven(temp)
.withHelloWorld()
.with("cache", cache)
Expand Down
Loading

5 comments on commit 99ed421

@0pdd
Copy link

@0pdd 0pdd commented on 99ed421 Jan 26, 2024

Choose a reason for hiding this comment

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

Puzzle 2746-9394bf60 disappeared from eo-maven-plugin/src/main/java/org/eolang/maven/optimization/OptCached.java), that's why I closed #2790. Please, remember that the puzzle was not necessarily removed in this particular commit. Maybe it happened earlier, but we discovered this fact only now.

@0pdd
Copy link

@0pdd 0pdd commented on 99ed421 Jan 26, 2024

Choose a reason for hiding this comment

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

Puzzle 2746-ac77dd68 disappeared from eo-maven-plugin/src/main/java/org/eolang/maven/optimization/OptCached.java), that's why I closed #2791. Please, remember that the puzzle was not necessarily removed in this particular commit. Maybe it happened earlier, but we discovered this fact only now.

@0pdd
Copy link

@0pdd 0pdd commented on 99ed421 Jan 26, 2024

Choose a reason for hiding this comment

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

Puzzle 2790-ef16f785 discovered in eo-maven-plugin/src/main/java/org/eolang/maven/optimization/OptCached.java) and submitted as #2820. Please, remember that the puzzle was not necessarily added in this particular commit. Maybe it was added earlier, but we discovered it only now.

@0pdd
Copy link

@0pdd 0pdd commented on 99ed421 Jan 26, 2024

Choose a reason for hiding this comment

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

Puzzle 2790-b1dbe786 discovered in eo-maven-plugin/src/test/java/org/eolang/maven/optimization/OptCachedTest.java) and submitted as #2821. Please, remember that the puzzle was not necessarily added in this particular commit. Maybe it was added earlier, but we discovered it only now.

@0pdd
Copy link

@0pdd 0pdd commented on 99ed421 Jan 26, 2024

Choose a reason for hiding this comment

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

Puzzle 2746-0e77f2c4 discovered in eo-maven-plugin/src/main/java/org/eolang/maven/optimization/OptCached.java) and submitted as #2822. Please, remember that the puzzle was not necessarily added in this particular commit. Maybe it was added earlier, but we discovered it only now.

Please sign in to comment.