Skip to content
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

Make various perf improvements #805

Merged
merged 7 commits into from
May 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ plugins {
id "jacoco"
id "com.github.spotbugs" version "4.6.0"
id "io.codearte.nexus-staging" version "0.21.0"
id "me.champeau.gradle.jmh" version "0.5.3"
id "me.champeau.jmh" version "0.6.4"
}

ext {
Expand Down
1 change: 0 additions & 1 deletion config/checkstyle/checkstyle.xml
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@
<!-- See http://checkstyle.sf.net/config_coding.html -->
<module name="EqualsHashCode"/>
<module name="IllegalInstantiation"/>
<module name="InnerAssignment"/>
<module name="MissingSwitchDefault"/>
<module name="SimplifyBooleanExpression"/>
<module name="SimplifyBooleanReturn"/>
Expand Down
7 changes: 7 additions & 0 deletions config/spotbugs/filter.xml
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,11 @@
<Class name="software.amazon.smithy.model.knowledge.NeighborProviderIndex"/>
<Bug pattern="NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE"/>
</Match>

<!-- Spotbugs for some reason isn't seeing that Objects.requireNonNull prevents a null return.
this is used when dereferencing a WeakReference to the Model. -->
<Match>
<Class name="software.amazon.smithy.model.knowledge.HttpBindingIndex"/>
<Bug pattern="NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE"/>
</Match>
</FindBugsFilter>
Original file line number Diff line number Diff line change
Expand Up @@ -32,25 +32,24 @@
import software.amazon.smithy.model.shapes.OperationShape;
import software.amazon.smithy.model.shapes.ResourceShape;
import software.amazon.smithy.model.shapes.ServiceShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.model.shapes.ToShapeId;
import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.utils.Pair;

/**
* Resolves and indexes the ARN templates for each resource in a service.
*/
public final class ArnIndex implements KnowledgeIndex {
private final Map<ShapeId, String> arnServices;
private final Map<ShapeId, String> arnServices = new HashMap<>();
private final Map<ShapeId, Map<ShapeId, ArnTrait>> templates;
private final Map<ShapeId, Map<ShapeId, ArnTrait>> effectiveArns = new HashMap<>();

public ArnIndex(Model model) {
// Pre-compute the ARN services.
arnServices = unmodifiableMap(model.shapes(ServiceShape.class)
.flatMap(shape -> Trait.flatMapStream(shape, ServiceTrait.class))
.map(pair -> Pair.of(pair.getLeft().getId(), resolveServiceArn(pair)))
.collect(Collectors.toMap(Pair::getLeft, Pair::getRight)));
for (Shape service : model.getServiceShapesWithTrait(ServiceTrait.class)) {
arnServices.put(service.getId(), service.expectTrait(ServiceTrait.class).getArnNamespace());
}

// Pre-compute all of the ArnTemplates in a service shape.
TopDownIndex topDownIndex = TopDownIndex.of(model);
Expand All @@ -73,15 +72,15 @@ public static ArnIndex of(Model model) {
return model.getKnowledge(ArnIndex.class, ArnIndex::new);
}

private static String resolveServiceArn(Pair<ServiceShape, ServiceTrait> pair) {
return pair.getRight().getArnNamespace();
}

private Pair<ShapeId, Map<ShapeId, ArnTrait>> compileServiceArns(TopDownIndex index, ServiceShape service) {
return Pair.of(service.getId(), unmodifiableMap(index.getContainedResources(service.getId()).stream()
.flatMap(resource -> Trait.flatMapStream(resource, ArnTrait.class))
.map(pair -> Pair.of(pair.getLeft().getId(), pair.getRight()))
.collect(Collectors.toMap(Pair::getLeft, Pair::getRight))));
Map<ShapeId, ArnTrait> mapping = new HashMap<>();
for (ResourceShape resource : index.getContainedResources(service.getId())) {
resource.getTrait(ArnTrait.class).ifPresent(arnTrait -> {
mapping.put(resource.getId(), arnTrait);
});
}

return Pair.of(service.getId(), Collections.unmodifiableMap(mapping));
}

private void compileEffectiveArns(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,22 @@
import static java.util.stream.Collectors.toList;
import static software.amazon.smithy.model.validation.ValidationUtils.tickedList;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.regex.Pattern;
import java.util.stream.Stream;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.shapes.ResourceShape;
import software.amazon.smithy.model.shapes.ServiceShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.model.validation.AbstractValidator;
import software.amazon.smithy.model.validation.ValidationEvent;
import software.amazon.smithy.utils.OptionalUtils;
import software.amazon.smithy.utils.Pair;
import software.amazon.smithy.utils.SmithyInternalApi;

/**
Expand All @@ -47,28 +47,33 @@ public final class ArnTemplateValidator extends AbstractValidator {
@Override
public List<ValidationEvent> validate(Model model) {
ArnIndex arnIndex = ArnIndex.of(model);
return model.shapes(ServiceShape.class)
.flatMap(service -> Trait.flatMapStream(service, ServiceTrait.class))
.flatMap(pair -> validateService(model, arnIndex, pair.getLeft()))
.collect(toList());
List<ValidationEvent> events = new ArrayList<>();
for (ServiceShape service : model.getServiceShapesWithTrait(ServiceTrait.class)) {
events.addAll(validateService(model, arnIndex, service));
}
return events;
}

private Stream<ValidationEvent> validateService(Model model, ArnIndex arnIndex, ServiceShape service) {
private List<ValidationEvent> validateService(Model model, ArnIndex arnIndex, ServiceShape service) {
// Make sure each ARN template contains relevant identifiers.
return arnIndex.getServiceResourceArns(service.getId()).entrySet().stream()
.flatMap(entry -> OptionalUtils.stream(model.getShape(entry.getKey())
.flatMap(Shape::asResourceShape)
.map(resource -> Pair.of(resource, entry.getValue()))))
.flatMap(pair -> validateResourceArn(pair.getLeft(), pair.getRight()));
List<ValidationEvent> events = new ArrayList<>();
for (Map.Entry<ShapeId, ArnTrait> entry : arnIndex.getServiceResourceArns(service.getId()).entrySet()) {
model.getShape(entry.getKey()).flatMap(Shape::asResourceShape).ifPresent(resource -> {
events.addAll(validateResourceArn(resource, entry.getValue()));
});
}
return events;
}

private Stream<ValidationEvent> validateResourceArn(ResourceShape resource, ArnTrait template) {
private List<ValidationEvent> validateResourceArn(ResourceShape resource, ArnTrait template) {
// Fail early on syntax error, otherwise, validate that the
// template correspond to identifiers.
return syntax(resource, template).map(Stream::of).orElseGet(() -> Stream.concat(
OptionalUtils.stream(enough(resource.getIdentifiers().keySet(), resource, template)),
OptionalUtils.stream(tooMuch(resource.getIdentifiers().keySet(), resource, template)))
);
return syntax(resource, template).map(Collections::singletonList).orElseGet(() -> {
List<ValidationEvent> events = new ArrayList<>();
enough(resource.getIdentifiers().keySet(), resource, template).ifPresent(events::add);
tooMuch(resource.getIdentifiers().keySet(), resource, template).ifPresent(events::add);
return events;
});
}

// Validates the syntax of each template.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,16 @@

package software.amazon.smithy.aws.traits;

import static java.util.stream.Collectors.toList;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.shapes.ServiceShape;
import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.model.validation.AbstractValidator;
import software.amazon.smithy.model.validation.ValidationEvent;
import software.amazon.smithy.utils.MapUtils;
import software.amazon.smithy.utils.OptionalUtils;
import software.amazon.smithy.utils.SmithyInternalApi;
import software.amazon.smithy.utils.StringUtils;

Expand All @@ -45,10 +42,11 @@ public final class EventSourceValidator extends AbstractValidator {

@Override
public List<ValidationEvent> validate(Model model) {
return model.shapes(ServiceShape.class)
.flatMap(service -> Trait.flatMapStream(service, ServiceTrait.class))
.flatMap(pair -> OptionalUtils.stream(validateService(pair.getLeft(), pair.getRight())))
.collect(toList());
List<ValidationEvent> events = new ArrayList<>();
for (ServiceShape service : model.getServiceShapesWithTrait(ServiceTrait.class)) {
validateService(service, service.expectTrait(ServiceTrait.class)).ifPresent(events::add);
}
return events;
}

private Optional<ValidationEvent> validateService(ServiceShape service, ServiceTrait trait) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@

package software.amazon.smithy.aws.traits;

import static java.util.stream.Collectors.toList;

import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
Expand All @@ -25,11 +23,9 @@
import java.util.regex.Pattern;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.shapes.ServiceShape;
import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.model.validation.AbstractValidator;
import software.amazon.smithy.model.validation.ValidationEvent;
import software.amazon.smithy.model.validation.ValidationUtils;
import software.amazon.smithy.utils.OptionalUtils;
import software.amazon.smithy.utils.SetUtils;
import software.amazon.smithy.utils.SmithyInternalApi;

Expand Down Expand Up @@ -73,10 +69,11 @@ public final class SdkServiceIdValidator extends AbstractValidator {

@Override
public List<ValidationEvent> validate(Model model) {
return model.shapes(ServiceShape.class)
.flatMap(service -> Trait.flatMapStream(service, ServiceTrait.class))
.flatMap(pair -> OptionalUtils.stream(validateService(pair.getLeft(), pair.getRight())))
.collect(toList());
List<ValidationEvent> events = new ArrayList<>();
for (ServiceShape service : model.getServiceShapesWithTrait(ServiceTrait.class)) {
validateService(service, service.expectTrait(ServiceTrait.class)).ifPresent(events::add);
}
return events;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.model.traits.ErrorTrait;
import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.model.transform.ModelTransformer;
import software.amazon.smithy.model.transform.ModelTransformerPlugin;
import software.amazon.smithy.utils.SmithyInternalApi;
Expand Down Expand Up @@ -62,16 +61,16 @@ public Model onRemove(ModelTransformer transformer, Collection<Shape> shapes, Mo
}

private Set<Shape> getServicesToUpdate(Model model, Set<ShapeId> removedOperations, Set<ShapeId> removedErrors) {
return model.shapes(ServiceShape.class)
.flatMap(service -> Trait.flatMapStream(service, ClientEndpointDiscoveryTrait.class))
.filter(pair -> removedOperations.contains(pair.getRight().getOperation())
|| removedErrors.contains(pair.getRight().getError()))
.map(pair -> {
ServiceShape.Builder builder = pair.getLeft().toBuilder();
builder.removeTrait(ClientEndpointDiscoveryTrait.ID);
return builder.build();
})
.collect(Collectors.toSet());
Set<Shape> result = new HashSet<>();
for (ServiceShape service : model.getServiceShapesWithTrait(ClientEndpointDiscoveryTrait.class)) {
ClientEndpointDiscoveryTrait trait = service.expectTrait(ClientEndpointDiscoveryTrait.class);
if (removedOperations.contains(trait.getOperation()) || removedErrors.contains(trait.getError())) {
ServiceShape.Builder builder = service.toBuilder();
builder.removeTrait(ClientEndpointDiscoveryTrait.ID);
result.add(builder.build());
}
}
return result;
}

private Set<Shape> getOperationsToUpdate(
Expand All @@ -89,16 +88,17 @@ private Set<Shape> getOperationsToUpdate(
.flatMap(service -> discoveryIndex.getEndpointDiscoveryOperations(service).stream())
.collect(Collectors.toSet());

return model.shapes(OperationShape.class)
// Get all endpoint discovery operations
.flatMap(operation -> Trait.flatMapStream(operation, ClientDiscoveredEndpointTrait.class))
// Only get the ones where discovery is optional, as it is safe to remove in that case
.filter(pair -> !pair.getRight().isRequired())
// Only get the ones that aren't still bound to a service that requires endpoint discovery
.filter(pair -> !stillBoundOperations.contains(pair.getLeft().getId()))
// Remove the trait
.map(pair -> pair.getLeft().toBuilder().removeTrait(ClientDiscoveredEndpointTrait.ID).build())
.collect(Collectors.toSet());
// Get all endpoint discovery operations
Set<Shape> result = new HashSet<>();
for (OperationShape operation : model.getOperationShapesWithTrait(ClientDiscoveredEndpointTrait.class)) {
ClientDiscoveredEndpointTrait trait = operation.expectTrait(ClientDiscoveredEndpointTrait.class);
// Only get the ones where discovery is optional, as it is safe to remove in that case.
// Only get the ones that aren't still bound to a service that requires endpoint discovery.
if (!trait.isRequired() && !stillBoundOperations.contains(operation.getId())) {
result.add(operation.toBuilder().removeTrait(ClientDiscoveredEndpointTrait.ID).build());
}
}
return result;
}

private Set<Shape> getMembersToUpdate(Model model, Set<ShapeId> updatedOperations) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.knowledge.KnowledgeIndex;
import software.amazon.smithy.model.knowledge.OperationIndex;
Expand All @@ -34,8 +33,6 @@
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.model.shapes.StructureShape;
import software.amazon.smithy.model.shapes.ToShapeId;
import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.utils.Pair;

public final class ClientEndpointDiscoveryIndex implements KnowledgeIndex {
private final Map<ShapeId, Map<ShapeId, ClientEndpointDiscoveryInfo>> endpointDiscoveryInfo = new HashMap<>();
Expand All @@ -44,26 +41,24 @@ public ClientEndpointDiscoveryIndex(Model model) {
TopDownIndex topDownIndex = TopDownIndex.of(model);
OperationIndex opIndex = OperationIndex.of(model);

model.shapes(ServiceShape.class)
.flatMap(service -> Trait.flatMapStream(service, ClientEndpointDiscoveryTrait.class))
.forEach(servicePair -> {
ServiceShape service = servicePair.getLeft();
ShapeId endpointOperationId = servicePair.getRight().getOperation();
ShapeId endpointErrorId = servicePair.getRight().getError();
for (ServiceShape service : model.getServiceShapesWithTrait(ClientEndpointDiscoveryTrait.class)) {
ClientEndpointDiscoveryTrait trait = service.expectTrait(ClientEndpointDiscoveryTrait.class);
ShapeId endpointOperationId = trait.getOperation();
ShapeId endpointErrorId = trait.getError();

Optional<OperationShape> endpointOperation = model.getShape(endpointOperationId)
.flatMap(Shape::asOperationShape);
Optional<StructureShape> endpointError = model.getShape(endpointErrorId)
.flatMap(Shape::asStructureShape);
Optional<OperationShape> endpointOperation = model.getShape(endpointOperationId)
.flatMap(Shape::asOperationShape);
Optional<StructureShape> endpointError = model.getShape(endpointErrorId)
.flatMap(Shape::asStructureShape);

if (endpointOperation.isPresent() && endpointError.isPresent()) {
Map<ShapeId, ClientEndpointDiscoveryInfo> serviceInfo = getOperations(
service, endpointOperation.get(), endpointError.get(), topDownIndex, opIndex);
if (!serviceInfo.isEmpty()) {
endpointDiscoveryInfo.put(service.getId(), serviceInfo);
}
}
});
if (endpointOperation.isPresent() && endpointError.isPresent()) {
Map<ShapeId, ClientEndpointDiscoveryInfo> serviceInfo = getOperations(
service, endpointOperation.get(), endpointError.get(), topDownIndex, opIndex);
if (!serviceInfo.isEmpty()) {
endpointDiscoveryInfo.put(service.getId(), serviceInfo);
}
}
}
}

public static ClientEndpointDiscoveryIndex of(Model model) {
Expand All @@ -77,22 +72,22 @@ private Map<ShapeId, ClientEndpointDiscoveryInfo> getOperations(
TopDownIndex topDownIndex,
OperationIndex opIndex
) {
return topDownIndex.getContainedOperations(service).stream()
.flatMap(operation -> Trait.flatMapStream(operation, ClientDiscoveredEndpointTrait.class))
.map(pair -> {
OperationShape operation = pair.getLeft();
List<MemberShape> discoveryIds = getDiscoveryIds(opIndex, operation);
ClientEndpointDiscoveryInfo info = new ClientEndpointDiscoveryInfo(
service,
operation,
endpointOperation,
endpointError,
discoveryIds,
pair.getRight().isRequired()
);
return Pair.of(operation.getId(), info);
})
.collect(Collectors.toMap(Pair::getLeft, Pair::getRight));
Map<ShapeId, ClientEndpointDiscoveryInfo> result = new HashMap<>();
for (OperationShape operation : topDownIndex.getContainedOperations(service)) {
operation.getTrait(ClientDiscoveredEndpointTrait.class).ifPresent(trait -> {
List<MemberShape> discoveryIds = getDiscoveryIds(opIndex, operation);
ClientEndpointDiscoveryInfo info = new ClientEndpointDiscoveryInfo(
service,
operation,
endpointOperation,
endpointError,
discoveryIds,
trait.isRequired()
);
result.put(operation.getId(), info);
});
}
return result;
}

private List<MemberShape> getDiscoveryIds(OperationIndex opIndex, OperationShape operation) {
Expand Down
Loading