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

matchUntilHalt() uses a lot of CPU? #68

Closed
devijvers opened this Issue Aug 31, 2013 · 3 comments

Comments

Projects
None yet
2 participants
@devijvers

devijvers commented Aug 31, 2013

I've noticed that both in Node.js and in the browser matchUntilHalt() uses a lot of CPU. Is this observation correct and if so how can it be alleviated?

@devijvers

This comment has been minimized.

Show comment
Hide comment
@devijvers

devijvers Sep 1, 2013

I've investigated this further. My rules aren't finished yet. There's one rule that modifies a fact for which there is no matching rule. When this last rule is invoked and the fact is modified Nools peaks at 100% with matchUntilHalt. To be clear, there is no halt() call anywhere in my rules.

When I apply this modification - and I don't modify my rules - the peak CPU problem goes away:

var finished = true;
var doMatch = function doMatch() {
  if (finished) {
    finished = false;
    console.log("restarting ...");
    session.match(function(err) {
      finished = true;
      console.log("ending ...");
      if (err) {
        console.error("Error");
        console.error(err);
        throw err;
      }
    });
  }
}
var modify_fn = session.modify;
session.modify = function modify(o, cb) {
  modify_fn.call(session, o, cb);
  doMatch();
}

doMatch();

/*session.matchUntilHalt(function(err) {
  if (err) {
    console.error("Error");
    console.error(err);
  }
});*/

devijvers commented Sep 1, 2013

I've investigated this further. My rules aren't finished yet. There's one rule that modifies a fact for which there is no matching rule. When this last rule is invoked and the fact is modified Nools peaks at 100% with matchUntilHalt. To be clear, there is no halt() call anywhere in my rules.

When I apply this modification - and I don't modify my rules - the peak CPU problem goes away:

var finished = true;
var doMatch = function doMatch() {
  if (finished) {
    finished = false;
    console.log("restarting ...");
    session.match(function(err) {
      finished = true;
      console.log("ending ...");
      if (err) {
        console.error("Error");
        console.error(err);
        throw err;
      }
    });
  }
}
var modify_fn = session.modify;
session.modify = function modify(o, cb) {
  modify_fn.call(session, o, cb);
  doMatch();
}

doMatch();

/*session.matchUntilHalt(function(err) {
  if (err) {
    console.error("Error");
    console.error(err);
  }
});*/

@ghost ghost assigned doug-martin Sep 1, 2013

@doug-martin

This comment has been minimized.

Show comment
Hide comment
@doug-martin

doug-martin Sep 1, 2013

Contributor

Yep so the way matchUntilHalt works currently is naive in that it just essentially does a async loop using setImmediate for each loop causing the CPU usage to be high. I think what you have above could be adapted to monitor any assert, retract, or modify to do the looping.

Contributor

doug-martin commented Sep 1, 2013

Yep so the way matchUntilHalt works currently is naive in that it just essentially does a async loop using setImmediate for each loop causing the CPU usage to be high. I think what you have above could be adapted to monitor any assert, retract, or modify to do the looping.

@devijvers

This comment has been minimized.

Show comment
Hide comment
@devijvers

devijvers Sep 1, 2013

I agree. assert/retract/modify should be a signal that match needs to restart.

There's also an infinite loop - also with the regular match when there is more than one rule that applies. I think the same applies here: there's no point for match to keep verifying until assert/retract/modify have been fired.

Come to think of it, I guess I could have used the emitted events as well.

devijvers commented Sep 1, 2013

I agree. assert/retract/modify should be a signal that match needs to restart.

There's also an infinite loop - also with the regular match when there is more than one rule that applies. I think the same applies here: there's no point for match to keep verifying until assert/retract/modify have been fired.

Come to think of it, I guess I could have used the emitted events as well.

doug-martin added a commit to doug-martin/nools that referenced this issue Sep 24, 2013

v0.1.13
* Fixed issue #68 where `matchUntilHalt` uses a lot of CPU
* Fixed issue #45, now compiled rules support `or` constraint with more than 2 inner constraints.
* Added new feature to address #76, now you can use `deleteFlows` to dispose all flows, or use `hasFlow` to check if a flow is already registred with `nools`.

@doug-martin doug-martin referenced this issue Sep 24, 2013

Merged

v0.1.13 #77

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment