Skip to content

Commit

Permalink
fix: do not invalidate implicit copybook cache on re-analysis
Browse files Browse the repository at this point in the history
Do not invalidate implicit copybook cache on re-analysis. But these needs to be invalidated if configuration changes happen. Also, when a dialec is registered

Signed-off-by: Aman Prashant <aman.prashant@broadcom.com>
  • Loading branch information
ap891843 committed Jun 28, 2024
1 parent 6a25155 commit f473617
Show file tree
Hide file tree
Showing 14 changed files with 119 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@
package org.eclipse.lsp.cobol.common.copybook;

import java.util.Collection;
import java.util.Collections;
import java.util.Set;

import com.google.common.collect.ImmutableSet;
import lombok.NonNull;
import org.eclipse.lsp.cobol.common.CleanerPreprocessor;
import org.eclipse.lsp.cobol.common.ResultWithErrors;
Expand All @@ -30,7 +27,7 @@
public interface CopybookService {
String FILE_BASENAME_VARIABLE = "${fileBasenameNoExtension}";
/** Remove all the stored copybook. */
void invalidateCache();
void invalidateCache(boolean onlyNonImplicit);

void invalidateCache(CopybookId copybookId);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package org.eclipse.lsp.cobol.core.preprocessor.delegates;

import com.google.inject.Inject;
import java.util.ArrayList;
import java.util.List;
import lombok.NonNull;
import org.antlr.v4.runtime.BufferedTokenStream;
import org.antlr.v4.runtime.CharStreams;
Expand All @@ -34,9 +36,6 @@
import org.eclipse.lsp.cobol.core.preprocessor.delegates.replacement.ReplacePreprocessorFactory;
import org.eclipse.lsp.cobol.core.semantics.CopybooksRepository;

import java.util.ArrayList;
import java.util.List;

/**
* This class runs pre-processing for COBOL using CobolPreprocessor.g4 grammar file. As a result, it
* returns an extended document with all the available copybooks included, with their definitions
Expand Down Expand Up @@ -84,6 +83,7 @@ private ResultWithErrors<CopybooksRepository> preprocess(

GrammarPreprocessorListener<CopybooksRepository> listener = listenerFactory.create(context, preprocessor);

ThreadInterruptionUtil.checkThreadInterrupted();
CobolPreprocessor parser = new CobolPreprocessor(tokens);
parser.removeErrorListeners();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.eclipse.lsp.cobol.common.error.SyntaxError;
import org.eclipse.lsp.cobol.common.message.MessageService;
import org.eclipse.lsp.cobol.common.model.Locality;
import org.eclipse.lsp.cobol.common.utils.ThreadInterruptionUtil;
import org.eclipse.lsp.cobol.core.CobolPreprocessorBaseListener;
import org.eclipse.lsp.cobol.core.preprocessor.delegates.GrammarPreprocessor;
import org.eclipse.lsp.cobol.core.preprocessor.delegates.PreprocessorContext;
Expand Down Expand Up @@ -175,6 +176,12 @@ private boolean requiresEarlyReturn(ParserRuleContext ctx) {
return false;
}

@Override
public void enterEveryRule(ParserRuleContext ctx) {
ThreadInterruptionUtil.checkThreadInterrupted();
super.enterEveryRule(ctx);
}

@Override
public void enterReplaceAreaStart(ReplaceAreaStartContext ctx) {
Locality locality = preprocessorService.retrieveLocality(ctx);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,15 @@
*/
package org.eclipse.lsp.cobol.core.visitor;

import static java.util.Optional.ofNullable;
import static java.util.stream.Collectors.toList;
import static org.eclipse.lsp.cobol.common.OutlineNodeNames.FILLER_NAME;
import static org.eclipse.lsp.cobol.core.CobolDataDivisionParser.*;

import com.google.common.collect.ImmutableList;
import java.util.*;
import java.util.function.Function;
import javax.annotation.Nonnull;
import lombok.extern.slf4j.Slf4j;
import org.antlr.v4.runtime.ParserRuleContext;
import org.antlr.v4.runtime.RuleContext;
Expand All @@ -24,22 +32,13 @@
import org.antlr.v4.runtime.tree.TerminalNode;
import org.eclipse.lsp.cobol.common.model.Locality;
import org.eclipse.lsp.cobol.common.model.tree.Node;
import org.eclipse.lsp.cobol.core.CobolDataDivisionParser;
import org.eclipse.lsp.cobol.common.model.tree.variable.ValueInterval;
import org.eclipse.lsp.cobol.common.model.tree.variable.UsageFormat;
import org.eclipse.lsp.cobol.common.model.tree.variable.ValueInterval;
import org.eclipse.lsp.cobol.core.CobolDataDivisionParser;
import org.eclipse.lsp.cobol.core.CobolParser;
import org.eclipse.lsp4j.Position;
import org.eclipse.lsp4j.Range;

import javax.annotation.Nonnull;
import java.util.*;
import java.util.function.Function;

import static java.util.Optional.ofNullable;
import static java.util.stream.Collectors.toList;
import static org.eclipse.lsp.cobol.core.CobolDataDivisionParser.*;
import static org.eclipse.lsp.cobol.common.OutlineNodeNames.FILLER_NAME;

/** Utility class for visitor and delegates classes with useful methods */
@Slf4j
//@UtilityClass
Expand Down Expand Up @@ -304,7 +303,7 @@ public static Function<Locality, List<Node>> constructNode(
* specific exception
*/
public static void checkInterruption() {
if (Thread.interrupted()) {
if (Thread.currentThread().isInterrupted()) {
LOG.debug("Parsing interrupted by user");
throw new ParseCancellationException("Parsing interrupted by user");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@

import com.google.common.collect.ImmutableList;
import java.util.*;
import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;
import java.util.Optional;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.Stream;
Expand Down Expand Up @@ -48,6 +52,7 @@
import org.eclipse.lsp.cobol.common.model.tree.StopNode;
import org.eclipse.lsp.cobol.common.model.tree.variable.QualifiedReferenceNode;
import org.eclipse.lsp.cobol.common.model.tree.variable.VariableUsageNode;
import org.eclipse.lsp.cobol.common.utils.ThreadInterruptionUtil;
import org.eclipse.lsp.cobol.implicitDialects.cics.nodes.ExecCicsHandleNode;
import org.eclipse.lsp.cobol.implicitDialects.cics.nodes.ExecCicsNode;
import org.eclipse.lsp.cobol.implicitDialects.cics.nodes.ExecCicsReturnNode;
Expand All @@ -56,11 +61,6 @@
import org.eclipse.lsp4j.Position;
import org.eclipse.lsp4j.Range;

import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;
import java.util.Optional;

/**
* This visitor analyzes the parser tree for CICS and returns its semantic context as a syntax tree
*/
Expand Down Expand Up @@ -195,6 +195,7 @@ public List<Node> visitParagraphNameUsage(CICSParser.ParagraphNameUsageContext c

@Override
public List<Node> visitChildren(RuleNode node) {
ThreadInterruptionUtil.checkThreadInterrupted();
return super.visitChildren(node);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,10 @@ public boolean isCopybook(String uri) {
copyUri = copyUri.replace(" ", "%20");
return new URL(decodeUri).sameFile(new URL(copyUri));
} catch (IOException e) {
if (ImplicitCodeUtils.isImplicit(copyUri)) {
LOG.debug("{} is a implicit copybook", copyUri);
return true;
}
LOG.error("IOException encountered while comparing paths {} and {}", copyUri, uri);
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ public void reanalyseOpenedPrograms() throws InterruptedException {
TimeUnit.MILLISECONDS.sleep(100);
} while (analysisInProgress);

copybookService.invalidateCache();
copybookService.invalidateCache(true);
subroutineService.invalidateCache();
LOG.info("Cache invalidated");
openDocuments
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.inject.Inject;
import java.util.List;
import lombok.extern.slf4j.Slf4j;
import org.eclipse.lsp.cobol.common.copybook.CopybookService;
import org.eclipse.lsp.cobol.common.message.LocaleStore;
import org.eclipse.lsp.cobol.common.message.MessageService;
import org.eclipse.lsp.cobol.common.utils.LogLevelUtils;
Expand All @@ -46,6 +47,7 @@ public class DidChangeConfigurationHandler {
private final MessageService messageService;
private final AsyncAnalysisService asyncAnalysisService;
private final CodeLayoutStore codeLayoutStore;
private final CopybookService copybookService;

@Inject
public DidChangeConfigurationHandler(DisposableLSPStateService disposableLSPStateService,
Expand All @@ -56,7 +58,8 @@ public DidChangeConfigurationHandler(DisposableLSPStateService disposableLSPStat
Keywords keywords,
MessageService messageService,
AsyncAnalysisService asyncAnalysisService,
CodeLayoutStore codeLayoutStore) {
CodeLayoutStore codeLayoutStore,
CopybookService copybookService) {
this.disposableLSPStateService = disposableLSPStateService;
this.settingsService = settingsService;
this.copybookNameService = copybookNameService;
Expand All @@ -66,6 +69,7 @@ public DidChangeConfigurationHandler(DisposableLSPStateService disposableLSPStat
this.messageService = messageService;
this.asyncAnalysisService = asyncAnalysisService;
this.codeLayoutStore = codeLayoutStore;
this.copybookService = copybookService;
}

/**
Expand All @@ -76,7 +80,7 @@ public void didChangeConfiguration(DidChangeConfigurationParams params) {
if (disposableLSPStateService.isServerShutdown()) {
return;
}

copybookService.invalidateCache(false);
messageService.reloadMessages();
copybookNameService
.copybookLocalFolders(null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import org.eclipse.lsp.cobol.common.copybook.CopybookId;
import org.eclipse.lsp.cobol.common.copybook.CopybookModel;
import org.eclipse.lsp.cobol.common.utils.ImplicitCodeUtils;

/**
* Implements copybook cache functionality
Expand Down Expand Up @@ -52,6 +54,15 @@ public void invalidateAll() {
cache.invalidateAll();
}

/** Invalidates only non-implicit copybook cache */
public void invalidateAllNonImplicit() {
cache.invalidateAll(
cache.asMap().entrySet().stream()
.filter(entry -> entry.getValue().getUri() != null)
.filter(entry -> !ImplicitCodeUtils.isImplicit(entry.getValue().getUri()))
.collect(Collectors.toList()));
}

/**
* Gets copybook model from cache
* @param copybookId copybook name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,17 @@ public CopybookServiceImpl(Provider<CobolLanguageClient> clientProvider,
}

@Override
public void invalidateCache() {
public void invalidateCache(boolean onlyNonImplicit) {
LOG.debug("Copybooks for downloading: {}", copybooksForDownloading);
LOG.debug("Copybook cache: {}", copybookCache);
LOG.debug("Cache invalidated");
copybookUsage.clear();
copybooksForDownloading.clear();
copybookCache.invalidateAll();
if (onlyNonImplicit) {
copybookCache.invalidateAllNonImplicit();
} else {
copybookCache.invalidateAll();
}
}

/**
Expand Down Expand Up @@ -180,9 +184,10 @@ private CopybookModel resolveSync(
}

private Optional<CopybookModel> tryResolvePredefinedCopybook(CopybookName copybookName) {
CopybookId copybookId = copybookName.toCopybookId(ImplicitCodeUtils.createFullUrl(copybookName.getDisplayName()));
CopybookName predefineCopybookName = new CopybookName(copybookName.getDisplayName().toUpperCase(), copybookName.getDialectType(), copybookName.getExtension());
CopybookId copybookId = predefineCopybookName.toCopybookId(ImplicitCodeUtils.createFullUrl(predefineCopybookName.getDisplayName()));
try {
CopybookModel copybookModel = copybookCache.get(copybookId, () -> new CopybookModel(copybookId, copybookName, null, null));
CopybookModel copybookModel = copybookCache.get(copybookId, () -> new CopybookModel(copybookId, predefineCopybookName, null, null));
if (copybookModel.getContent() == null || copybookModel.getUri() == null) return Optional.empty();
return Optional.of(copybookModel);
} catch (ExecutionException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ void testReanalyseOpenedPrograms() {
throw new RuntimeException(e);
}

verify(copybookService, times(1)).invalidateCache();
verify(copybookService, times(1)).invalidateCache(true);
verify(subroutineService, times(1)).invalidateCache();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import java.util.List;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;

import org.eclipse.lsp.cobol.common.copybook.CopybookService;
import org.eclipse.lsp.cobol.common.dialects.CobolLanguageId;
import org.eclipse.lsp.cobol.common.message.LocaleStore;
import org.eclipse.lsp.cobol.common.message.MessageService;
Expand All @@ -55,6 +55,7 @@ void testChangeConfigurationNoPathToRegister() throws InterruptedException {
DisposableLSPStateService stateService = new CobolLSPServerStateService();
SettingsService settingsService = mock(SettingsService.class);
WatcherService watchingService = mock(WatcherService.class);
CopybookService copybookService = mock(CopybookService.class);
LocaleStore localeStore = mock(LocaleStore.class);
CopybookNameService copybookNameService = mock(CopybookNameService.class);
Keywords keywords = mock(Keywords.class);
Expand All @@ -70,7 +71,7 @@ void testChangeConfigurationNoPathToRegister() throws InterruptedException {
localeStore,
keywords,
messageService,
asyncAnalysisService, getMockLayoutStore());
asyncAnalysisService, getMockLayoutStore(), copybookService);


when(copybookNameService.copybookLocalFolders(null))
Expand Down Expand Up @@ -101,6 +102,7 @@ void testChangeConfigurationNoChangesInPaths() throws InterruptedException {
Keywords keywords = mock(Keywords.class);
MessageService messageService = mock(MessageService.class);
AsyncAnalysisService asyncAnalysisService = mock(AsyncAnalysisService.class);
CopybookService copybookService = mock(CopybookService.class);

DidChangeConfigurationHandler didChangeConfigurationHandler =
new DidChangeConfigurationHandler(
Expand All @@ -112,7 +114,8 @@ void testChangeConfigurationNoChangesInPaths() throws InterruptedException {
keywords,
messageService,
asyncAnalysisService,
getMockLayoutStore());
getMockLayoutStore(),
copybookService);

String path = "foo/bar";

Expand Down Expand Up @@ -145,6 +148,7 @@ void testChangeConfigurationNewPath() throws InterruptedException {
Keywords keywords = mock(Keywords.class);
MessageService messageService = mock(MessageService.class);
AsyncAnalysisService asyncAnalysisService = mock(AsyncAnalysisService.class);
CopybookService copybookService = mock(CopybookService.class);

DidChangeConfigurationHandler didChangeConfigurationHandler =
new DidChangeConfigurationHandler(
Expand All @@ -156,7 +160,7 @@ void testChangeConfigurationNewPath() throws InterruptedException {
keywords,
messageService,
asyncAnalysisService,
getMockLayoutStore());
getMockLayoutStore(), copybookService);

ArgumentCaptor<List<String>> watcherCaptor = forClass(List.class);
String path = "foo/bar";
Expand Down Expand Up @@ -193,6 +197,7 @@ void testChangeConfigurationPathRemoved() throws InterruptedException {
Keywords keywords = mock(Keywords.class);
MessageService messageService = mock(MessageService.class);
AsyncAnalysisService asyncAnalysisService = mock(AsyncAnalysisService.class);
CopybookService copybookService = mock(CopybookService.class);

DidChangeConfigurationHandler didChangeConfigurationHandler =
new DidChangeConfigurationHandler(
Expand All @@ -204,7 +209,8 @@ void testChangeConfigurationPathRemoved() throws InterruptedException {
keywords,
messageService,
asyncAnalysisService,
getMockLayoutStore());
getMockLayoutStore(),
copybookService);
ArgumentCaptor<List<String>> watcherCaptor = forClass(List.class);
JsonArray arr = new JsonArray();
String path = "foo/bar";
Expand Down
Loading

0 comments on commit f473617

Please sign in to comment.