Skip to content

Commit

Permalink
Definition/Reference: Reduce space used (#6249)
Browse files Browse the repository at this point in the history 
SortedSet is complex in memory, serialized, JSON, computationally.

VI Model changes, but question answers stay the same.
  • Loading branch information
@dhalperi
dhalperi committed Sep 30, 2020
1 parent 38735cc commit 073d509
Show file tree
Hide file tree
Showing 24 changed files with 3,847 additions and 16,009 deletions.
33 changes: 27 additions & 6 deletions ...cts/batfish-common-protocol/src/main/java/org/batfish/datamodel/DefinedStructureInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,33 +5,54 @@

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.collect.Range;
import java.io.Serializable;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.stream.IntStream;
import javax.annotation.Nonnull;

public class DefinedStructureInfo implements Serializable {

private static final String PROP_DEFINITION_LINES = "definitionLines";
private static final String PROP_NUM_REFERRERS = "numReferrers";

@Nonnull private SortedSet<Integer> _definitionLines;
@Nonnull private IntegerSpace _definitionLines;
private int _numReferrers;

@JsonCreator
public DefinedStructureInfo(
@JsonProperty(PROP_DEFINITION_LINES) SortedSet<Integer> definitionLines,
@JsonProperty(PROP_DEFINITION_LINES) IntegerSpace definitionLines,
@JsonProperty(PROP_NUM_REFERRERS) Integer numReferrers) {
checkArgument(numReferrers != null, "Missing %s", PROP_NUM_REFERRERS);
_definitionLines = firstNonNull(definitionLines, new TreeSet<>());
_definitionLines = firstNonNull(definitionLines, IntegerSpace.EMPTY);
_numReferrers = numReferrers;
}

@JsonProperty(PROP_DEFINITION_LINES)
public SortedSet<Integer> getDefinitionLines() {
public @Nonnull IntegerSpace getDefinitionLines() {
return _definitionLines;
}

public void addDefinitionLines(int line) {
if (_definitionLines.contains(line)) {
return;
}
_definitionLines = _definitionLines.toBuilder().including(line).build();
}

public void addDefinitionLines(int... lines) {
_definitionLines = _definitionLines.toBuilder().including(lines).build();
}

public void addDefinitionLines(IntStream lines) {
IntegerSpace.Builder newLines = _definitionLines.toBuilder();
lines.forEach(newLines::including);
_definitionLines = newLines.build();
}

public void addDefinitionLines(Range<Integer> lines) {
_definitionLines = _definitionLines.toBuilder().including(lines).build();
}

@JsonProperty(PROP_NUM_REFERRERS)
public int getNumReferrers() {
return _numReferrers;
Expand Down
3 changes: 2 additions & 1 deletion projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/IntegerSpace.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import com.google.common.collect.RangeSet;
import java.util.Arrays;
import java.util.Set;
import java.util.SortedSet;
import java.util.stream.IntStream;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -69,7 +70,7 @@ protected IntegerSpace(RangeSet<Integer> rangeset) {

/** Return an ordered set of integers described by this space. */
@Override
public @Nonnull Set<Integer> enumerate() {
public @Nonnull SortedSet<Integer> enumerate() {
return ImmutableRangeSet.copyOf(_rangeset).asSet(DiscreteDomain.integers());
}

Expand Down
39 changes: 15 additions & 24 deletions ...otocol/src/main/java/org/batfish/datamodel/answers/ConvertConfigurationAnswerElement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@
import java.io.Serializable;
import java.util.Set;
import java.util.SortedMap;
import java.util.SortedSet;
import java.util.TreeMap;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.batfish.common.BatfishException;
import org.batfish.common.ErrorDetails;
import org.batfish.common.Warnings;
import org.batfish.datamodel.DefinedStructureInfo;
import org.batfish.datamodel.IntegerSpace;
import org.batfish.version.BatfishVersion;

/**
Expand Down Expand Up @@ -49,25 +49,24 @@ public class ConvertConfigurationAnswerElement extends InitStepAnswerElement
_definedStructures;

/* Map of source filename to generated nodes (e.g. "configs/j1.cfg" -> ["j1_master", "j1_logical_system1"]) */
@Nonnull private Multimap<String, String> _fileMap;
@Nonnull private final Multimap<String, String> _fileMap;

// filename -> structType -> structName -> usage -> lines
@Nonnull
private SortedMap<
String, SortedMap<String, SortedMap<String, SortedMap<String, SortedSet<Integer>>>>>
private final SortedMap<
String, SortedMap<String, SortedMap<String, SortedMap<String, IntegerSpace>>>>
_referencedStructures;

@Nonnull private SortedMap<String, BatfishException.BatfishStackTrace> _errors;

@Nonnull private SortedMap<String, ErrorDetails> _errorDetails;
@Nonnull private final SortedMap<String, ErrorDetails> _errorDetails;

// This is just to support legacy objects, before _convertStatus map was used
@Nullable private Set<String> _failed;

// filename -> structType -> structName -> usage -> lines
@Nonnull
private SortedMap<
String, SortedMap<String, SortedMap<String, SortedMap<String, SortedSet<Integer>>>>>
private SortedMap<String, SortedMap<String, SortedMap<String, SortedMap<String, IntegerSpace>>>>
_undefinedReferences;

@Nonnull private String _version;
Expand All @@ -85,17 +84,13 @@ public ConvertConfigurationAnswerElement() {
SortedMap<String, SortedMap<String, SortedMap<String, DefinedStructureInfo>>>
definedStructures,
@JsonProperty(PROP_REFERENCED_STRUCTURES)
SortedMap<
String,
SortedMap<String, SortedMap<String, SortedMap<String, SortedSet<Integer>>>>>
SortedMap<String, SortedMap<String, SortedMap<String, SortedMap<String, IntegerSpace>>>>
referencedstructures,
@JsonProperty(PROP_CONVERT_STATUS) SortedMap<String, ConvertStatus> convertStatus,
@JsonProperty(PROP_ERRORS) SortedMap<String, BatfishException.BatfishStackTrace> errors,
@JsonProperty(PROP_ERROR_DETAILS) SortedMap<String, ErrorDetails> errorDetails,
@JsonProperty(PROP_UNDEFINED_REFERENCES)
SortedMap<
String,
SortedMap<String, SortedMap<String, SortedMap<String, SortedSet<Integer>>>>>
SortedMap<String, SortedMap<String, SortedMap<String, SortedMap<String, IntegerSpace>>>>
undefinedReferences,
@JsonProperty(PROP_VERSION) String version,
@JsonProperty(PROP_WARNINGS) SortedMap<String, Warnings> warnings,
Expand Down Expand Up @@ -132,7 +127,7 @@ SortedMap<String, ConvertStatus> getConvertStatusProp() {
return ImmutableSortedMap.copyOf(_convertStatus);
} else if (_failed != null) {
ImmutableSortedMap.Builder<String, ConvertStatus> map = ImmutableSortedMap.naturalOrder();
_failed.stream().forEach(n -> map.put(n, ConvertStatus.FAILED));
_failed.forEach(n -> map.put(n, ConvertStatus.FAILED));
return map.build();
} else {
return ImmutableSortedMap.of();
Expand Down Expand Up @@ -160,16 +155,14 @@ public Multimap<String, String> getFileMap() {

@JsonProperty(PROP_REFERENCED_STRUCTURES)
@Nonnull
public SortedMap<
String, SortedMap<String, SortedMap<String, SortedMap<String, SortedSet<Integer>>>>>
public SortedMap<String, SortedMap<String, SortedMap<String, SortedMap<String, IntegerSpace>>>>
getReferencedStructures() {
return _referencedStructures;
}

@JsonProperty(PROP_UNDEFINED_REFERENCES)
@Nonnull
public SortedMap<
String, SortedMap<String, SortedMap<String, SortedMap<String, SortedSet<Integer>>>>>
public SortedMap<String, SortedMap<String, SortedMap<String, SortedMap<String, IntegerSpace>>>>
getUndefinedReferences() {
return _undefinedReferences;
}
Expand Down Expand Up @@ -201,7 +194,7 @@ public void setDefinedStructures(
}

@Override
public void setErrors(SortedMap<String, BatfishException.BatfishStackTrace> errors) {
public void setErrors(@Nonnull SortedMap<String, BatfishException.BatfishStackTrace> errors) {
_errors = errors;
}

Expand All @@ -213,19 +206,17 @@ void setFailed(Set<String> failed) {

public void setUndefinedReferences(
@Nonnull
SortedMap<
String,
SortedMap<String, SortedMap<String, SortedMap<String, SortedSet<Integer>>>>>
SortedMap<String, SortedMap<String, SortedMap<String, SortedMap<String, IntegerSpace>>>>
undefinedReferences) {
_undefinedReferences = undefinedReferences;
}

public void setVersion(String version) {
public void setVersion(@Nonnull String version) {
_version = version;
}

@Override
public void setWarnings(SortedMap<String, Warnings> warnings) {
public void setWarnings(@Nonnull SortedMap<String, Warnings> warnings) {
_warnings = warnings;
}
}
69 changes: 42 additions & 27 deletions projects/batfish-common-protocol/src/main/java/org/batfish/vendor/VendorConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Multiset;
import com.google.common.collect.Range;
import com.google.common.collect.SortedMultiset;
import com.google.common.collect.TreeMultiset;
import java.io.Serializable;
Expand All @@ -16,9 +17,8 @@
import java.util.Objects;
import java.util.Set;
import java.util.SortedMap;
import java.util.SortedSet;
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.stream.IntStream;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.antlr.v4.runtime.ParserRuleContext;
Expand All @@ -32,6 +32,7 @@
import org.batfish.datamodel.Configuration;
import org.batfish.datamodel.ConfigurationFormat;
import org.batfish.datamodel.DefinedStructureInfo;
import org.batfish.datamodel.IntegerSpace;
import org.batfish.datamodel.answers.ConvertConfigurationAnswerElement;
import org.batfish.datamodel.isp_configuration.IspConfiguration;
import org.batfish.grammar.BatfishCombinedParser;
Expand Down Expand Up @@ -219,41 +220,64 @@ public abstract List<Configuration> toVendorIndependentConfigurations()
throws VendorConversionException;

private void addStructureReference(
SortedMap<String, SortedMap<String, SortedMap<String, SortedMap<String, SortedSet<Integer>>>>>
SortedMap<String, SortedMap<String, SortedMap<String, SortedMap<String, IntegerSpace>>>>
referenceMap,
StructureType structureType,
String name,
StructureUsage usage,
int line) {
String filename = getFilename();
SortedMap<String, SortedMap<String, SortedMap<String, SortedSet<Integer>>>> byType =
SortedMap<String, SortedMap<String, SortedMap<String, IntegerSpace>>> byType =
referenceMap.computeIfAbsent(filename, k -> new TreeMap<>());
String type = structureType.getDescription();
SortedMap<String, SortedMap<String, SortedSet<Integer>>> byName =
SortedMap<String, SortedMap<String, IntegerSpace>> byName =
byType.computeIfAbsent(type, k -> new TreeMap<>());
SortedMap<String, SortedSet<Integer>> byUsage =
byName.computeIfAbsent(name, k -> new TreeMap<>());
SortedMap<String, IntegerSpace> byUsage = byName.computeIfAbsent(name, k -> new TreeMap<>());
String usageStr = usage.getDescription();
SortedSet<Integer> lines = byUsage.computeIfAbsent(usageStr, k -> new TreeSet<>());
lines.add(line);
byUsage.compute(
usageStr,
(ignored, refs) ->
(refs == null ? IntegerSpace.of(line) : refs.toBuilder().including(line).build()));
}

public void undefined(StructureType structureType, String name, StructureUsage usage, int line) {
addStructureReference(
_answerElement.getUndefinedReferences(), structureType, name, usage, line);
}

/* Recursively process children to find all relevant definition lines for the specified context */
private static IntStream collectLines(RuleContext ctx, BatfishCombinedParser<?, ?> parser) {
return IntStream.range(0, ctx.getChildCount())
.flatMap(
i -> {
ParseTree child = ctx.getChild(i);
if (child instanceof TerminalNode) {
return IntStream.of(parser.getLine(((TerminalNode) child).getSymbol()));
} else if (child instanceof RuleContext) {
return collectLines((RuleContext) child, parser);
}
return IntStream.empty();
})
.distinct();
}

/**
* Updates structure definitions to include the specified structure {@code name} and {@code
* structureType} and initializes the number of referrers.
* Gets the {@link DefinedStructureInfo} for the specified structure {@code name} and {@code
* structureType}, initializing if necessary.
*/
public void defineSingleLineStructure(StructureType structureType, String name, int line) {
private DefinedStructureInfo getStructureInfo(StructureType structureType, String name) {
String type = structureType.getDescription();
SortedMap<String, DefinedStructureInfo> byName =
_structureDefinitions.computeIfAbsent(type, k -> new TreeMap<>());
DefinedStructureInfo info =
byName.computeIfAbsent(name, k -> new DefinedStructureInfo(new TreeSet<>(), 0));
info.getDefinitionLines().add(line);
return byName.computeIfAbsent(name, k -> new DefinedStructureInfo(IntegerSpace.EMPTY, 0));
}

/**
* Updates structure definitions to include the specified structure {@code name} and {@code
* structureType} and initializes the number of referrers.
*/
public void defineSingleLineStructure(StructureType structureType, String name, int line) {
getStructureInfo(structureType, name).addDefinitionLines(line);
}

/**
Expand All @@ -267,15 +291,7 @@ public void defineSingleLineStructure(StructureType structureType, String name,
*/
public void defineFlattenedStructure(
StructureType type, String name, RuleContext ctx, BatfishCombinedParser<?, ?> parser) {
/* Recursively process children to find all relevant definition lines for the specified context */
for (int i = 0; i < ctx.getChildCount(); i++) {
ParseTree child = ctx.getChild(i);
if (child instanceof TerminalNode) {
defineSingleLineStructure(type, name, parser.getLine(((TerminalNode) child).getSymbol()));
} else if (child instanceof RuleContext) {
defineFlattenedStructure(type, name, (RuleContext) child, parser);
}
}
getStructureInfo(type, name).addDefinitionLines(collectLines(ctx, parser));
}

/**
Expand All @@ -286,9 +302,8 @@ public void defineFlattenedStructure(
* RuleContext, BatfishCombinedParser)}.
*/
public void defineStructure(StructureType type, String name, ParserRuleContext ctx) {
for (int i = ctx.getStart().getLine(); i <= ctx.getStop().getLine(); ++i) {
defineSingleLineStructure(type, name, i);
}
getStructureInfo(type, name)
.addDefinitionLines(Range.closed(ctx.getStart().getLine(), ctx.getStop().getLine()));
}

/**
Expand Down

0 comments on commit 073d509

Please sign in to comment.