Skip to content

Commit

Permalink
Improve BottomUpIndex performance
Browse files Browse the repository at this point in the history
When doing some ad-hoc profiling on the language server using aws
service models I noticed that BottomUpIndex was taking up a lot of CPU
time. This was because it used selectors and PathFinder to determine the
paths from resources/operations up to their enclosing service. Selectors
are pretty slow in comparison to regular java code, and when the model
is large with many services/operations, this index can take a while to
compute. BottomUpIndex is used for validating `@cfnResource` (through
CfnResourcePropertyValidator -> CfnResourceIndex), so anyone using this
trait is paying this performance cost (in particular, SDK code
generators).

To get a rough idea of how much faster this commit makes BottomUpIndex,
I ran model validation using the cli on all aws service models with and
without this change a few times each:
```
smithy validate --quiet # 14 - 16 seconds

/path/to/smithy/smithy-cli/build/image/smithy-cli-darwin-aarch64/bin/smithy
validate --quiet # 8 - 9 seconds
```
  • Loading branch information
milesziemer committed Aug 27, 2024
1 parent 5f07427 commit 8a34fbf
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 20 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package software.amazon.smithy.model.jmh;

import java.util.concurrent.TimeUnit;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.Warmup;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.knowledge.BottomUpIndex;

@Warmup(iterations = 3)
@Measurement(iterations = 3, timeUnit = TimeUnit.MICROSECONDS)
@BenchmarkMode(Mode.AverageTime)
@Fork(1)
public class KnowledgeIndicies {
@State(Scope.Thread)
public static class IndiciesState {
public Model model;

@Setup
public void prepare() {
model = Model.assembler()
.addImport(KnowledgeIndicies.class.getResource("test-model.smithy"))
.disableValidation()
.assemble()
.getResult()
.get();
}
}

@Benchmark
public BottomUpIndex createsBottomUpIndex(IndiciesState state) {
// Use the ctor so the index doesn't get cached in warmup runs and re-used
return new BottomUpIndex(state.model);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,47 +15,62 @@

package software.amazon.smithy.model.knowledge;

import java.util.ArrayList;
import java.util.ArrayDeque;
import java.util.Collections;
import java.util.Deque;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.neighbor.NeighborProvider;
import software.amazon.smithy.model.neighbor.Relationship;
import software.amazon.smithy.model.selector.PathFinder;
import software.amazon.smithy.model.selector.Selector;
import software.amazon.smithy.model.shapes.EntityShape;
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.utils.ListUtils;

/**
* Computes all of the parent shapes of resources and operations from the bottom-up.
*/
public final class BottomUpIndex implements KnowledgeIndex {
private static final Selector SELECTOR = Selector.parse(":is(resource, operation)");
private final Map<ShapeId, Map<ShapeId, List<EntityShape>>> parentBindings = new HashMap<>();

public BottomUpIndex(Model model) {
PathFinder pathFinder = PathFinder.create(model);

NeighborProvider provider = NeighborProviderIndex.of(model).getProvider();
for (ServiceShape service : model.getServiceShapes()) {
Map<ShapeId, List<EntityShape>> serviceBindings = new HashMap<>();
parentBindings.put(service.getId(), serviceBindings);
for (PathFinder.Path path : pathFinder.search(service, SELECTOR)) {
List<EntityShape> shapes = new ArrayList<>();
// Get all of the path elements other than the last one in reverse order.
for (int i = path.size() - 1; i >= 0; i--) {
Relationship rel = path.get(i);
// This should always be an EntityShape, but just in case new relationships are added...
if (rel.getShape() instanceof EntityShape) {
shapes.add((EntityShape) rel.getShape());
}
}
// Add the end shape (a resource or operation) to the service bindings.
serviceBindings.put(path.getEndShape().getId(), shapes);
Map<ShapeId, List<EntityShape>> paths = new HashMap<>();
parentBindings.put(service.getId(), paths);

Deque<EntityShape> path = new ArrayDeque<>();
path.push(service);
collectPaths(paths, path, service, provider);
}
}

private void collectPaths(
Map<ShapeId, List<EntityShape>> paths,
Deque<EntityShape> path,
Shape current,
NeighborProvider neighborProvider
) {
for (Relationship relationship : neighborProvider.getNeighbors(current)) {
Shape neighbor = relationship.expectNeighborShape();
if (!neighbor.isOperationShape() && !neighbor.isResourceShape()) {
continue;
}

// Note: The path does not include the neighbor shape
paths.put(neighbor.getId(), ListUtils.copyOf(path));

// Recurse through neighbors of an entity (resource) shape
if (neighbor instanceof EntityShape) {
path.push((EntityShape) neighbor);
collectPaths(paths, path, neighbor, neighborProvider);
path.pop();
}
}
}
Expand Down

0 comments on commit 8a34fbf

Please sign in to comment.