Add predicate mapping cost analyzer

src/api/mapping/support/PredicateMappingCostAnalyzer.ts

@@ -0,0 +1,31 @@
+/*
+ * Copyright © 2018 Atomist, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { PredicateMapping } from "../PredicateMapping";
+
+/**
+ * Classification of expected PredicateMapping cost
+ */
+export enum ExpectedPredicateMappingCost {
+    cheap = "cheap",
+    expensive = "expensive",
+    unknown = "unknown",
+}
+
+/**
+ * Function that can classify PredicateMappings by expected cost to evaluate
+ */
+export type PredicateMappingCostAnalyzer<F> = (pm: PredicateMapping<F>) => ExpectedPredicateMappingCost;

src/api/mapping/support/defaultPredicateMappingCostAnalyzer.ts

@@ -0,0 +1,61 @@
+/*
+ * Copyright © 2018 Atomist, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { logger } from "@atomist/automation-client";
+import { PredicateMapping } from "../PredicateMapping";
+import { ExpectedPredicateMappingCost, PredicateMappingCostAnalyzer } from "./PredicateMappingCostAnalyzer";
+
+/**
+ * Indications that evaluating this test may be expensive
+ */
+const ExpensiveSigns: Array<(pm: PredicateMapping<any>) => boolean> = [
+    pm => mappingCodeIncludes(pm, "await "),
+    pm => mappingCodeIncludes(pm, ".project"),
+    pm => mappingCodeIncludes(pm, ".graphClient"),
+    pm => mappingCodeIncludes(pm, ".totalFileCount"),
+    pm => mappingCodeIncludes(pm, ".doWithFiles"),
+    pm => mappingCodeIncludes(pm, ".getFile", ".findFile"),
+    pm => mappingCodeIncludes(pm, ".stream"),
+    pm => mappingCodeIncludes(pm, "findMatches", "findFileMatches", "doWithMatches", "doWithFileMatches"),
+];
+
+/**
+ * Estimate cost by looking at code to see if it goes through a project
+ * @param {PredicateMapping<any>} pm
+ * @return {any}
+ * @constructor
+ */
+export const DefaultPredicateMappingCostAnalyzer: PredicateMappingCostAnalyzer<any> =
+    pm => {
+        const mappingCode = pm.mapping.toString();
+        if (ExpensiveSigns.some(sign => sign(pm))) {
+            logger.info("Expected cost of [%s] is expensive", mappingCode);
+            return ExpectedPredicateMappingCost.expensive;
+        }
+        logger.info("Expected cost of [%s] is unknown", mappingCode);
+        return ExpectedPredicateMappingCost.unknown;
+    };
+
+/**
+ * Does the mapping include any of these patterns
+ * @param {PredicateMapping<any>} pm
+ * @param {string} patterns
+ * @return {boolean}
+ */
+function mappingCodeIncludes(pm: PredicateMapping<any>, ...patterns: string[]): boolean {
+    const code = pm.mapping.toString();
+    return patterns.some(pattern => code.includes(pattern));
+}

src/api/mapping/support/predicateUtils.ts

@@ -16,6 +16,8 @@
 
 import { logger } from "@atomist/automation-client";
 import { PredicateMapping } from "../PredicateMapping";
+import { DefaultPredicateMappingCostAnalyzer } from "./defaultPredicateMappingCostAnalyzer";
+import { ExpectedPredicateMappingCost, PredicateMappingCostAnalyzer } from "./PredicateMappingCostAnalyzer";
 
 /**
  * Return the opposite of this predicate mapping
@@ -31,43 +33,76 @@ export function whenNot<F>(t: PredicateMapping<F>): PredicateMapping<F> {
  * Wrap all these predicates in a single predicate
  * AND: Return true if all are satisfied
  * @param {PredicateMapping} predicates
+ * @param analyzer analyzer to use for performance optimization
  * @return {PredicateMapping}
  */
-export function all<F>(...predicates: Array<PredicateMapping<F>>): PredicateMapping<F> {
+export function all<F>(predicates: Array<PredicateMapping<F>>,
+                       analyzer: PredicateMappingCostAnalyzer<F> = DefaultPredicateMappingCostAnalyzer): PredicateMapping<F> {
     return {
         name: predicates.map(g => g.name).join(" && "),
-        mapping: async pci => {
-            const allResults: boolean[] = await Promise.all(
-                predicates.map(async pt => {
-                    const result = await pt.mapping(pci);
-                    logger.debug(`Result of PushTest '${pt.name}' was ${result}`);
-                    return result;
-                }),
-            );
-            return !allResults.includes(false);
-        },
+        mapping: async pci => optimizedAndEvaluation(predicates, analyzer)(pci),
     };
 }
 
 /**
  * Wrap all these predicates in a single predicate
  * OR: Return true if any is satisfied
  * @param {PredicateMapping} predicates
+ * @param analyzer analyzer to use for performance optimization
  * @return {PredicateMapping}
  */
-export function any<F>(...predicates: Array<PredicateMapping<F>>): PredicateMapping<F> {
+export function any<F>(predicates: Array<PredicateMapping<F>>,
+                       analyzer: PredicateMappingCostAnalyzer<F> = DefaultPredicateMappingCostAnalyzer): PredicateMapping<F> {
     return {
         name: predicates.map(g => g.name).join(" || "),
         mapping: async pci => {
-            const allResults: boolean[] = await Promise.all(
-                predicates.map(async pt => {
-                    const result = await pt.mapping(pci);
-                    logger.debug(`Result of PushTest '${pt.name}' was ${result}`);
-                    return result;
-                }),
-            );
+            // Cannot short-circuit this
+            const allResults: boolean[] = await gatherResults(predicates)(pci);
             return allResults.includes(true);
         },
     };
 
 }
+
+/**
+ * Evaluate predicates for an AND, running non-expensive ones first
+ * @param {Array<PredicateMapping<F>>} predicates
+ * @param {PredicateMappingCostAnalyzer<F>} analyzer
+ * @return {(f: F) => Promise<boolean>}
+ */
+function optimizedAndEvaluation<F>(predicates: Array<PredicateMapping<F>>,
+                                   analyzer: PredicateMappingCostAnalyzer<F>): (f: F) => Promise<boolean> {
+    const cheap: Array<PredicateMapping<F>> = [];
+    const remaining: Array<PredicateMapping<F>> = [];
+
+    for (const p of predicates) {
+        const cost = analyzer(p);
+        if (cost !== ExpectedPredicateMappingCost.expensive) {
+            cheap.push(p);
+        } else {
+            remaining.push(p);
+        }
+    }
+    logger.info("Cheap: [%j], remaining: [%j]", cheap, remaining);
+
+    return async pci => {
+        const cheapResults = await gatherResults(cheap)(pci);
+        if (cheapResults.includes(false)) {
+            return false;
+        }
+        const remainingResults = await gatherResults(remaining)(pci);
+        return !remainingResults.includes(false);
+    };
+}
+
+function gatherResults<F>(predicates: Array<PredicateMapping<F>>): (f: F) => Promise<boolean[]> {
+    return pci => {
+        return Promise.all(
+            predicates.map(async pt => {
+                const result = await pt.mapping(pci);
+                logger.debug(`Result of PushTest '${pt.name}' was ${result}`);
+                return result;
+            }),
+        );
+    };
+}

src/api/mapping/support/projectPredicateUtils.ts

@@ -33,7 +33,7 @@ export function notPredicate(t: ProjectPredicate): ProjectPredicate {
  * @return {ProjectPredicate}
  */
 export function allPredicatesSatisfied(...predicates: ProjectPredicate[]): ProjectPredicate {
-    return all(...predicates.map(toPredicateMapping)).mapping;
+    return all(predicates.map(toPredicateMapping)).mapping;
 }
 
 /**
@@ -43,7 +43,7 @@ export function allPredicatesSatisfied(...predicates: ProjectPredicate[]): Proje
  * @return {ProjectPredicate}
  */
 export function anyPredicateSatisfied(...predicates: ProjectPredicate[]): ProjectPredicate {
-    return any(...predicates.map(toPredicateMapping)).mapping;
+    return any(predicates.map(toPredicateMapping)).mapping;
 }
 
 function toPredicateMapping(p: ProjectPredicate, i: number): PredicateMapping<Project> {

src/api/mapping/support/pushTestUtils.ts

@@ -41,7 +41,7 @@ export type PushTestOrProjectPredicate = PushTest | ProjectPredicate;
  */
 export function allSatisfied(...pushTests: PushTestOrProjectPredicate[]): PushTest {
     const asPushTests = pushTests.map(p => isMapping(p) ? p : predicatePushTest(p.name, p));
-    return pred.all(...asPushTests);
+    return pred.all(asPushTests);
 }
 
 /**
@@ -52,7 +52,7 @@ export function allSatisfied(...pushTests: PushTestOrProjectPredicate[]): PushTe
  */
 export function anySatisfied(...pushTests: PushTestOrProjectPredicate[]): PushTest {
     const asPushTests = pushTests.map(p => isMapping(p) ? p : predicatePushTest(p.name, p));
-    return pred.any(...asPushTests);
+    return pred.any(asPushTests);
 }
 
 const pushTestResultMemory = new LruCache<boolean>(1000);

test/api/mapping/support/projectPredicateUtilsTest.ts

@@ -17,12 +17,13 @@
 import { GitHubRepoRef } from "@atomist/automation-client/operations/common/GitHubRepoRef";
 import { Project } from "@atomist/automation-client/project/Project";
 
+import { InMemoryProject } from "@atomist/automation-client/project/mem/InMemoryProject";
 import * as assert from "power-assert";
 import {
     allPredicatesSatisfied,
     anyPredicateSatisfied, notPredicate,
 } from "../../../../src/api/mapping/support/projectPredicateUtils";
-import { FalseProjectPredicate, TrueProjectPredicate } from "./pushTestUtilsTest";
+import { BigProject, FalseProjectPredicate, TrueProjectPredicate } from "./pushTestUtilsTest";
 
 const id = new GitHubRepoRef("atomist", "github-sdm");
 
@@ -31,56 +32,75 @@ describe("projectPredicatesUtilsTest", () => {
     describe("not", () => {
 
         it("should handle one true", async () => {
-            const r = await notPredicate(TrueProjectPredicate)({id} as any as Project);
+            const r = await notPredicate(TrueProjectPredicate)({ id } as any as Project);
             assert(!r);
         });
 
         it("should handle one false", async () => {
-            const r = await notPredicate(FalseProjectPredicate)({id} as any as Project);
+            const r = await notPredicate(FalseProjectPredicate)({ id } as any as Project);
             assert(r);
         });
 
     });
 
     describe("allPredicatesSatisfied", () => {
 
-            it("should handle one true", async () => {
-                const r = await allPredicatesSatisfied(TrueProjectPredicate)({id} as any as Project);
-                assert(r);
-            });
+        it("should handle one true", async () => {
+            const r = await allPredicatesSatisfied(TrueProjectPredicate)({ id } as any as Project);
+            assert(r);
+        });
 
-            it("should handle two true", async () => {
-                const r = await allPredicatesSatisfied(TrueProjectPredicate, TrueProjectPredicate)({id} as any as Project);
-                assert(r);
-            });
+        it("should handle two true", async () => {
+            const r = await allPredicatesSatisfied(TrueProjectPredicate, TrueProjectPredicate)({ id } as any as Project);
+            assert(r);
+        });
 
-            it("should handle one true and one false", async () => {
-                const r = await allPredicatesSatisfied(TrueProjectPredicate, FalseProjectPredicate)({id} as any as Project);
-                assert(!r);
-            });
+        it("should handle one true and one false", async () => {
+            const r = await allPredicatesSatisfied(TrueProjectPredicate, FalseProjectPredicate)({ id } as any as Project);
+            assert(!r);
         });
 
+        it("should handle one true and one false, and not evaluate slow predicate that will fail", async () => {
+            const r = await allPredicatesSatisfied(
+                TrueProjectPredicate,
+                FalseProjectPredicate,
+                BigProject)({ id } as any as Project);
+            assert(!r);
+        });
+
+        it("should eventually evaluate slow predicate", async () => {
+            const p = InMemoryProject.from(id);
+            for (let i = 0; i < 500; i++) {
+                p.addFileSync(`path-${i}`, "content");
+            }
+            const r = await allPredicatesSatisfied(
+                TrueProjectPredicate,
+                BigProject)(p);
+            assert(r);
+        });
+    });
+
     describe("anyPredicateSatisfied", () => {
 
-            it("should handle one true", async () => {
-                const r = await anyPredicateSatisfied(TrueProjectPredicate)({id} as any as Project);
-                assert(r);
-            });
-
-            it("should handle two true", async () => {
-                const r = await anyPredicateSatisfied(TrueProjectPredicate, TrueProjectPredicate)({id} as any as Project);
-                assert(r);
-            });
-
-            it("should handle one true and one false", async () => {
-                const r = await anyPredicateSatisfied(TrueProjectPredicate, FalseProjectPredicate)({id} as any as Project);
-                assert(r);
-            });
-
-            it("should handle two false", async () => {
-                const r = await anyPredicateSatisfied(FalseProjectPredicate, FalseProjectPredicate)({id} as any as Project);
-                assert(!r);
-            });
+        it("should handle one true", async () => {
+            const r = await anyPredicateSatisfied(TrueProjectPredicate)({ id } as any as Project);
+            assert(r);
+        });
+
+        it("should handle two true", async () => {
+            const r = await anyPredicateSatisfied(TrueProjectPredicate, TrueProjectPredicate)({ id } as any as Project);
+            assert(r);
+        });
+
+        it("should handle one true and one false", async () => {
+            const r = await anyPredicateSatisfied(TrueProjectPredicate, FalseProjectPredicate)({ id } as any as Project);
+            assert(r);
+        });
+
+        it("should handle two false", async () => {
+            const r = await anyPredicateSatisfied(FalseProjectPredicate, FalseProjectPredicate)({ id } as any as Project);
+            assert(!r);
+        });
 
     });
 

test/api/mapping/support/pushTestUtilsTest.ts

@@ -29,6 +29,8 @@ export const TrueProjectPredicate: ProjectPredicate = async () => true;
 
 export const FalseProjectPredicate: ProjectPredicate = async () => false;
 
+export const BigProject: ProjectPredicate = async p => (await p.totalFileCount()) > 100;
+
 const id = new GitHubRepoRef("atomist", "github-sdm");
 
 describe("pushTestUtilsTest", () => {