PasupuletiRohini commented 2 years ago

Hi,

We are trying to use the latest version of library 4.1.0 but we are facing a performance issue with the DefaultRulesEngine#fire method from version 4.0.0 I referred to the below test code from the issue https://github.com/j-easy/easy-rules/issues/169 and there is a lot of difference in time taken between versions 3.4.0 and 4.0.0, 4.1.0 Our default logging level is WARN so I think it is not a problem with logging. Could you help me in fixing the issue.

import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;

import org.jeasy.rules.api.Facts;
import org.jeasy.rules.api.Rule;
import org.jeasy.rules.api.Rules;
import org.jeasy.rules.api.RulesEngine;
import org.jeasy.rules.core.DefaultRulesEngine;
import org.jeasy.rules.core.RuleBuilder;

public class RuleEnginePerfTest {

    private static Rules RULES_1 = new Rules();
    private static int NO_OF_REQUESTS = 50;
    private static int NO_OF_RULES = 50;
    private static RulesEngine ruleEngine;

    public static void main(String[] args) {
        ruleEngine = new DefaultRulesEngine();
        loadRules();
        fireRules();
    }

    private static void fireRules() {
        ThreadPoolExecutor executor = new ThreadPoolExecutor(100, 200, 100, TimeUnit.SECONDS, new ArrayBlockingQueue<>(100));
        Facts facts = new Facts();

        facts.put("proxyUrl", "easy/v7/api");
        ruleEngine.fire(RULES_1, facts);

        for (int i = 0; i < NO_OF_REQUESTS; i++) {
            Wokrer worker1 = new Wokrer(ruleEngine, RULES_1, facts);
            executor.submit(worker1);
        }
        executor.shutdown();
        System.out.println("Finished all threads");
    }

    private static void loadRules() {
        for (int i = 0; i < NO_OF_RULES; i++) {
            String condtion = "easy/v"+i+"/api";
            String action = "Execute for rule" + i;
            Rule rule = new RuleBuilder()
                        .name("weather rule" + i)
                        .priority(NO_OF_RULES - i)
                        .description("if it rains then take an umbrella")
                        .when(facts -> facts.get("proxyUrl").equals(condtion))
                        .then(facts -> System.out.println(action))
                        .build();
            RULES_1.register(rule);
        }
    }

}

class Wokrer implements Runnable {

    RulesEngine ruleEngine;
    Rules rules;
    Facts facts;

    public Wokrer(RulesEngine ruleEngine, Rules rules, Facts facts) {
        this.ruleEngine = ruleEngine;
        this.rules = rules;
        this.facts = facts;
    }

    @Override
    public void run() {
        long startTime = System.currentTimeMillis();
        ruleEngine.fire(rules, facts);
        System.out.println("Time Taken in ms:"+ (System.currentTimeMillis()-startTime));
    }

}

code-uri commented 2 years ago

Watching this issue. let me know if you guys found something.

zhhaojie commented 2 years ago

I ran your code on my MacBook about 10 times and got the result. Model Name: MacBook Pro Chip: Apple M1 Pro Total Number of Cores: 10 (8 performance and 2 efficiency) Memory: 32 GB JDK: java 11

v3.4.0 Time Taken in ms:43 Time Taken in ms:34 Time Taken in ms:52 Time Taken in ms:41 Time Taken in ms:1 Time Taken in ms:42 Time Taken in ms:1 Time Taken in ms:29 Time Taken in ms:46 Time Taken in ms:46 Time Taken in ms:40 Time Taken in ms:35 Time Taken in ms:2 Time Taken in ms:1 Time Taken in ms:1 Time Taken in ms:41 Time Taken in ms:39 Time Taken in ms:2 Time Taken in ms:44 Time Taken in ms:3 Time Taken in ms:33 Time Taken in ms:40 Time Taken in ms:34 Time Taken in ms:44 Time Taken in ms:3 Time Taken in ms:38 Time Taken in ms:1 Time Taken in ms:0 Time Taken in ms:4 Time Taken in ms:33 Time Taken in ms:39 Time Taken in ms:2 Time Taken in ms:41 Time Taken in ms:32 Time Taken in ms:42 Time Taken in ms:43 Time Taken in ms:35 Time Taken in ms:49 Time Taken in ms:0 Time Taken in ms:34 Time Taken in ms:36 Time Taken in ms:38 Time Taken in ms:31 Time Taken in ms:33 Time Taken in ms:37 Time Taken in ms:0 Time Taken in ms:32 Time Taken in ms:40 Time Taken in ms:35 Time Taken in ms:38

v4.1.0 Time Taken in ms:866 Time Taken in ms:777 Time Taken in ms:873 Time Taken in ms:870 Time Taken in ms:858 Time Taken in ms:865 Time Taken in ms:851 Time Taken in ms:867 Time Taken in ms:859 Time Taken in ms:874 Time Taken in ms:863 Time Taken in ms:826 Time Taken in ms:867 Time Taken in ms:871 Time Taken in ms:858 Time Taken in ms:877 Time Taken in ms:862 Time Taken in ms:831 Time Taken in ms:858 Time Taken in ms:874 Time Taken in ms:831 Time Taken in ms:862 Time Taken in ms:868 Time Taken in ms:871 Time Taken in ms:864 Time Taken in ms:867 Time Taken in ms:765 Time Taken in ms:866 Time Taken in ms:856 Time Taken in ms:874 Time Taken in ms:860 Time Taken in ms:865 Time Taken in ms:869 Time Taken in ms:873 Time Taken in ms:870 Time Taken in ms:860 Time Taken in ms:835 Time Taken in ms:840 Time Taken in ms:859 Time Taken in ms:872 Time Taken in ms:799 Time Taken in ms:873 Time Taken in ms:871 Time Taken in ms:860 Time Taken in ms:870 Time Taken in ms:860 Time Taken in ms:867 Time Taken in ms:751 Time Taken in ms:845 Time Taken in ms:860

dvgaba commented 1 year ago

Is this still an issue for you? If yes, could you please try against below fork

<dependency>
    <groupId>io.github.dvgaba</groupId>
    <artifactId>easy-rules-core</artifactId>
    <version>1.0.5</version>
</dependency>

Time Taken in ms:1
Time Taken in ms:2
Time Taken in ms:3
Time Taken in ms:2
Time Taken in ms:1
Time Taken in ms:0
Time Taken in ms:1
Time Taken in ms:1
Time Taken in ms:1
Time Taken in ms:1
Time Taken in ms:3
Time Taken in ms:1
Time Taken in ms:1
Time Taken in ms:1
Time Taken in ms:2
Time Taken in ms:3
Time Taken in ms:1
Time Taken in ms:3
Time Taken in ms:2
Time Taken in ms:2
Time Taken in ms:2
Time Taken in ms:3
Time Taken in ms:3
Time Taken in ms:1
Time Taken in ms:1
Time Taken in ms:4
Time Taken in ms:3
Time Taken in ms:4
Time Taken in ms:2
Time Taken in ms:1
Time Taken in ms:2
Time Taken in ms:3
Time Taken in ms:1
Time Taken in ms:1
Time Taken in ms:6
Time Taken in ms:6
Time Taken in ms:6
Time Taken in ms:5
Time Taken in ms:6
Time Taken in ms:6
Time Taken in ms:6
Finished all threads
Time Taken in ms:6
Time Taken in ms:0
Time Taken in ms:6
Time Taken in ms:2
Time Taken in ms:2
Time Taken in ms:1
Time Taken in ms:0
Time Taken in ms:3
Time Taken in ms:1

spattanaik75 commented 1 year ago

i tried multiple versions without any luck. ended up writing a tiny version of the rules engine.


public enum MyRuleSet {
    MY_RULESET_2_1(
            "MY-RULESET-2.1",
            "some description of the rule",
            1,
            x -> true,
            x -> x.setSomething(MappingConstants.MY_CONSTANT)
    ),

   MY_RULESET_2_33(
            "MY-RULESET-2.33",
            "some description",
            2,
            x -> x.getValue().length() == 0
                    && Strings.isNullOrEmpty(x.getName()),
            x -> x.setCollateralIssuerSector(
                    Constants.getSarbCodeByCode(x.getSomeValue(), "default"))
    ),

    private final String ruleId;
    private final String ruleDesc;
    private final Integer priority;
    private final Predicate<CanonicalSecured> condition;
    private final Consumer<CanonicalSecured> action;
}

// driver code


X runAllRules(X x) {

        final int PRIORITY_THRESHOLD = 5;

        IntStream.range(1, PRIORITY_THRESHOLD+1).forEach(
                priority -> {
                    for (MyRuleSet rule : MyRuleSet.values()) {
                        if (priority == rule.getPriority() && rule.getCondition().test(x))
                            rule.getAction().accept(x);

                    }
                }

        );

        return x;
    }

here the input is only one variable. you can customize accordingly.

-s

dvgaba commented 1 year ago

I did couple of minor improvements in logging statements, I think it should perform very well now. If you have a sample I can re-visit.

Maven central

<dependency>
    <groupId>io.github.dvgaba</groupId>
    <artifactId>easy-rules-core</artifactId>
    <version>1.0.5</version>
</dependency>

amondnet commented 1 year ago

I looked at the changes between 3.4.0 and 4.0.0. As a result, I figured out that facts were changed from map to set.

3.4.0

    public <T> T get(String name) {
        Objects.requireNonNull(name);
        return (T) facts.get(name);
    }

4.0.0

    public Fact<?> getFact(String factName) {
        Objects.requireNonNull(factName, "fact name must not be null");
        return facts.stream()
                .filter(fact -> fact.getName().equals(factName))
                .findFirst()
                .orElse(null);
    }

That is, the more calls to facts.get() the slower it gets.

3.4.0

246292.207 ops/s

Result "com.balancefriends.rules.jmh.Benchmarks.fire": 242826.285 ±(99.9%) 5934.515 ops/s [Average] (min, avg, max) = (234522.671, 242826.285, 247286.446), stdev = 3925.314 CI (99.9%): [236891.770, 248760.800] (assumes normal distribution)

Run complete. Total time: 00:00:50

REMEMBER: The numbers below are just data. To gain reusable insights, you need to follow up on why the numbers are the way they are. Use profilers (see -prof, -lprof), design factorial experiments, perform baseline and negative tests that provide experimental control, make sure the benchmarking environment is safe on JVM/OS/HW level, ask for reviews from the domain experts. Do not assume the numbers tell you what you want them to tell.

Benchmark Mode Cnt Score Error Units Benchmarks.fire thrpt 10 242826.285 ± 5934.515 ops/s

4.1.0

121724.376 ops/s

Result "com.balancefriends.rules.jmh.Benchmarks.fire": 119130.799 ±(99.9%) 2603.791 ops/s [Average] (min, avg, max) = (116331.019, 119130.799, 121724.376), stdev = 1722.247 CI (99.9%): [116527.008, 121734.590] (assumes normal distribution)

Run complete. Total time: 00:00:50

REMEMBER: The numbers below are just data. To gain reusable insights, you need to follow up on why the numbers are the way they are. Use profilers (see -prof, -lprof), design factorial experiments, perform baseline and negative tests that provide experimental control, make sure the benchmarking environment is safe on JVM/OS/HW level, ask for reviews from the domain experts. Do not assume the numbers tell you what you want them to tell.

Benchmark Mode Cnt Score Error Units Benchmarks.fire thrpt 10 119130.799 ± 2603.791 ops/s

io.github.dvgaba:easy-rules-core:1.0.5

126121.153 ops/s

Result "com.balancefriends.rules.jmh.Benchmarks.fire": 123466.592 ±(99.9%) 3073.242 ops/s [Average] (min, avg, max) = (119931.789, 123466.592, 126121.153), stdev = 2032.759 CI (99.9%): [120393.351, 126539.834] (assumes normal distribution)

Run complete. Total time: 00:00:50

REMEMBER: The numbers below are just data. To gain reusable insights, you need to follow up on why the numbers are the way they are. Use profilers (see -prof, -lprof), design factorial experiments, perform baseline and negative tests that provide experimental control, make sure the benchmarking environment is safe on JVM/OS/HW level, ask for reviews from the domain experts. Do not assume the numbers tell you what you want them to tell.

Benchmark Mode Cnt Score Error Units Benchmarks.fire thrpt 10 123466.592 ± 3073.242 ops/s

4.1.0 without `facts.get`

    rulesEngine = new DefaultRulesEngine();
    rules = new Rules();
    for (int i = 0; i < NO_OF_RULES; i++) {
      String condition = "easy/v"+i+"/api";
      String action = "Execute for rule" + i;
      int finalI = i;
      Rule rule = new RuleBuilder()
          .name("weather rule" + i)
          .priority(NO_OF_RULES - i)
          .description("if it rains then take an umbrella")
          .when(facts -> finalI == 7)
          .then(facts -> System.out.println(action))
          .build();
      rules.register(rule);
    }

228171.310 ops/s

Result "com.balancefriends.rules.jmh.Benchmarks.fire": 228503.124 ±(99.9%) 7976.100 ops/s [Average] (min, avg, max) = (218975.159, 228503.124, 234008.662), stdev = 5275.696 CI (99.9%): [220527.024, 236479.224] (assumes normal distribution)

Run complete. Total time: 00:00:50

REMEMBER: The numbers below are just data. To gain reusable insights, you need to follow up on why the numbers are the way they are. Use profilers (see -prof, -lprof), design factorial experiments, perform baseline and negative tests that provide experimental control, make sure the benchmarking environment is safe on JVM/OS/HW level, ask for reviews from the domain experts. Do not assume the numbers tell you what you want them to tell.

Benchmark Mode Cnt Score Error Units Benchmarks.fire thrpt 10 228503.124 ± 7976.100 ops/s

4.1.0 with refactor `Facts.java`

/*
 * The MIT License
 *
 *  Copyright (c) 2020, Mahmoud Ben Hassine (mahmoud.benhassine@icloud.com)
 *
 *  Permission is hereby granted, free of charge, to any person obtaining a copy
 *  of this software and associated documentation files (the "Software"), to deal
 *  in the Software without restriction, including without limitation the rights
 *  to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
 *  copies of the Software, and to permit persons to whom the Software is
 *  furnished to do so, subject to the following conditions:
 *
 *  The above copyright notice and this permission notice shall be included in
 *  all copies or substantial portions of the Software.
 *
 *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 *  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
 *  AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 *  LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 *  OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 *  THE SOFTWARE.
 */
package org.jeasy.rules.api;

import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Objects;
import java.util.Set;

/**
 * This class encapsulates a set of facts and represents a facts namespace.
 * Facts have unique names within a <code>Facts</code> object.
 *
 * @author Mahmoud Ben Hassine (mahmoud.benhassine@icloud.com)
 */
public class Facts implements Iterable<Fact<?>> {

    private final Map<String, Fact<?>> facts = new HashMap<>();

    /**
     * Add a fact, replacing any fact with the same name.
     *
     * @param name of the fact to add, must not be null
     * @param value of the fact to add, must not be null
     */
    public <T> void put(String name, T value) {
        Objects.requireNonNull(name, "fact name must not be null");
        Objects.requireNonNull(value, "fact value must not be null");
        Fact<?> retrievedFact = getFact(name);
        if (retrievedFact != null) {
            remove(retrievedFact);
        }
        add(new Fact<>(name, value));
    }

    /**
     * Add a fact, replacing any fact with the same name.
     * 
     * @param fact to add, must not be null
     */
    public <T> void add(Fact<T> fact) {
        Objects.requireNonNull(fact, "fact must not be null");
        Fact<?> retrievedFact = getFact(fact.getName());
        if (retrievedFact != null) {
            remove(retrievedFact);
        }
        facts.put(fact.getName(), fact);
    }

    /**
     * Remove a fact by name.
     *
     * @param factName name of the fact to remove, must not be null
     */
    public void remove(String factName) {
        Objects.requireNonNull(factName, "fact name must not be null");
        Fact<?> fact = getFact(factName);
        if (fact != null) {
            remove(fact);
        }
    }

    /**
     * Remove a fact.
     *
     * @param fact to remove, must not be null
     */
    public <T> void remove(Fact<T> fact) {
        Objects.requireNonNull(fact, "fact must not be null");
        facts.remove(fact);
    }

    /**
     * Get the value of a fact by its name. This is a convenience method provided
     * as a short version of {@code getFact(factName).getValue()}.
     *
     * @param factName name of the fact, must not be null
     * @param <T> type of the fact's value
     * @return the value of the fact having the given name, or null if there is
     * no fact with the given name
     */
    @SuppressWarnings("unchecked")
    public <T> T get(String factName) {
        Objects.requireNonNull(factName, "fact name must not be null");
        Fact<?> fact = getFact(factName);
        if (fact != null) {
            return (T) fact.getValue();
        }
        return null;
    }

    /**
     * Get a fact by name.
     *
     * @param factName name of the fact, must not be null
     * @return the fact having the given name, or null if there is no fact with the given name
     */
    public Fact<?> getFact(String factName) {
        Objects.requireNonNull(factName, "fact name must not be null");
        return facts.get(factName);
    }

    /**
     * Return a copy of the facts as a map. It is not intended to manipulate
     * facts outside of the rules engine (aka other than manipulating them through rules).
     *
     * @return a copy of the current facts as a {@link HashMap}
     */
    public Map<String, Object> asMap() {
        Map<String, Object> map = new HashMap<>();
        for (Fact<?> fact : facts.values()) {
            map.put(fact.getName(), fact.getValue());
        }
        return map;
    }

    /**
     * Return an iterator on the set of facts. It is not intended to remove
     * facts using this iterator outside of the rules engine (aka other than doing it through rules)
     * 
     * @return an iterator on the set of facts
     */
    @Override
    public Iterator<Fact<?>> iterator() {
        return facts.values().iterator();
    }

    /**
     * Clear facts.
     */
    public void clear() {
        facts.clear();
    }

    @Override
    public String toString() {
        Iterator<Fact<?>> iterator = facts.values().iterator();
        StringBuilder stringBuilder = new StringBuilder("[");
        while (iterator.hasNext()) {
            stringBuilder.append(iterator.next().toString());
            if (iterator.hasNext()) {
                stringBuilder.append(",");
            }
        }
        stringBuilder.append("]");
        return stringBuilder.toString();
    }
}

205699.427 ops/s

Result "com.balancefriends.rules.jmh.Benchmarks.fire": 204850.475 ±(99.9%) 2609.767 ops/s [Average] (min, avg, max) = (202078.476, 204850.475, 207818.300), stdev = 1726.199 CI (99.9%): [202240.708, 207460.242] (assumes normal distribution)

Run complete. Total time: 00:00:50

REMEMBER: The numbers below are just data. To gain reusable insights, you need to follow up on why the numbers are the way they are. Use profilers (see -prof, -lprof), design factorial experiments, perform baseline and negative tests that provide experimental control, make sure the benchmarking environment is safe on JVM/OS/HW level, ask for reviews from the domain experts. Do not assume the numbers tell you what you want them to tell.

Benchmark Mode Cnt Score Error Units Benchmarks.fire thrpt 10 204850.475 ± 2609.767 ops/

dvgaba commented 1 year ago

This make sense, thanks for looking into it. I will make these changes in next patch release in my fork.

j-easy / easy-rules

Performance issue from the version 4.0.0 #379

3.4.0

Run complete. Total time: 00:00:50

4.1.0

Run complete. Total time: 00:00:50

io.github.dvgaba:easy-rules-core:1.0.5

Run complete. Total time: 00:00:50

4.1.0 without `facts.get`

Run complete. Total time: 00:00:50

4.1.0 with refactor `Facts.java`

Run complete. Total time: 00:00:50

j-easy / easy-rules

Performance issue from the version 4.0.0 #379

3.4.0

Run complete. Total time: 00:00:50

4.1.0

Run complete. Total time: 00:00:50

io.github.dvgaba:easy-rules-core:1.0.5

Run complete. Total time: 00:00:50

4.1.0 without facts.get

Run complete. Total time: 00:00:50

4.1.0 with refactor Facts.java

Run complete. Total time: 00:00:50

4.1.0 without `facts.get`

4.1.0 with refactor `Facts.java`