Re: [rules-dev] [rules-users] Starting engine using fireUntilHalt and inserting no facts results in 50% CPU usage
1:17 a.m.
Here's a diff for the minimal change I think fixes the problem:
Index: src/main/java/org/drools/common/DefaultAgenda.java
===================================================================
--- src/main/java/org/drools/common/DefaultAgenda.java (revision 35727)
+++ src/main/java/org/drools/common/DefaultAgenda.java (working copy)
@@ -1048,10 +1048,10 @@
boolean fired = fireNextItem( agendaFilter );
fired = fired || !((AbstractWorkingMemory)
this.workingMemory).getActionQueue().isEmpty();
this.workingMemory.executeQueuedActions();
- if ( !fired && !missedNotifyAll.get()) {
+ if ( !fired ) {
try {
synchronized ( this.halt ) {
- this.halt.wait();
+ if(!missedNotifyAll.get()) this.halt.wait();
}
} catch ( InterruptedException e ) {
this.halt.set( true );
Basically, move the missedNotifyAll check immediately before the wait. This still avoids
the race condition, and only causes one spurious trip through the fireUntilHalt loop on
startup. I'd open a JIRA with this in it but I need to get to sleep. :)
--- On Tue, 10/26/10, Greg Barton <greg_barton(a)yahoo.com> wrote:
From: Greg Barton <greg_barton(a)yahoo.com>
Subject: Re: [rules-users] Starting engine using fireUntilHalt and inserting no facts
results in 50% CPU usage
To: "Rules Users List" <rules-users(a)lists.jboss.org>,
rules-dev(a)lists.jboss.org
Date: Tuesday, October 26, 2010, 1:01 AM
The problem is the race condition fix
introduced in rev 30163. The missedNotifyAll flag is
set to true immediately which causes the busy loop.
missedNotifyAll is initialized to false, but is immediately
set to true because notifyHalt() is called in
AbstractWorkingMemory.queueWorkingMemoryAction() via
ReteooRuleBase.newStatefulSession(). Anyway, I'll move
discussions of possible solutions over to the dev list,
(crossposting now) or a JIRA if you'd prefer that.
--- On Mon, 10/25/10, Greg Barton <greg_barton(a)yahoo.com>
wrote:
> From: Greg Barton <greg_barton(a)yahoo.com>
> Subject: Re: [rules-users] Starting engine using
fireUntilHalt and inserting no facts results in 50% CPU
usage
> To: "Rules Users List" <rules-users(a)lists.jboss.org>
> Date: Monday, October 25, 2010, 3:18 PM
> I was going to look at this at some
> point. I guess that point will be tonight. :)
>
> GreG
>
> On Oct 25, 2010, at 15:02, Edson Tirelli <tirelli(a)post.com>
> wrote:
>
> No worries, it is a good starting point anyway.
> Hopefully someone
> can continue from there or I will eventually fix it.
Just a
> lot of
> stuff in my plate already that have priority over
this.
>
> Edson
>
> 2010/10/25 mardo <mardo(a)abicola.de>:
> Unfortunately no, I just had a look how it was
implemented
> (like
> http://tutorials.jenkov.com/java-concurrency/thread-signaling.html#missedsig
> nals) -> missed signals and noticed that it
somehow
> didn't work as intended.
>
> But no idea how to directly correct it, sorry.
>
> -----Original Message-----
> From: rules-users-bounces(a)lists.jboss.org
> [mailto:rules-users-bounces@lists.jboss.org]
> On Behalf Of Edson Tirelli
> Sent: Montag, 25. Oktober 2010 21:30
> To: Rules Users List
> Subject: Re: [rules-users] Starting engine using
> fireUntilHalt and inserting
> no facts results in 50% CPU usage
>
> Do you have a solution/patch for it?
>
> Edson
>
> 2010/10/25 mardo <mardo(a)abicola.de>:
> Luckily just stumbled upon this mail...
>
> You can take a look at
>
> http://drools-java-rules-engine.46999.n3.nabble.com/fireUntilHalt-and-OSGi-C
> PU-load-td1022352.html
>
> where I already provided a minimal example (don't know
if
> it covers all
> cases) and tracked it down to missedNotifyAll in
> DefaultAgenda
>
> cheers
>
>
>
> -----Original Message-----
> From: rules-users-bounces(a)lists.jboss.org
> [mailto:rules-users-bounces@lists.jboss.org]
> On Behalf Of Edson Tirelli
> Sent: Montag, 25. Oktober 2010 21:17
> To: Rules Users List
> Subject: Re: [rules-users] Starting engine using
> fireUntilHalt and
> inserting
> no facts results in 50% CPU usage
>
> This is clearly a regression as Drools should be
not be
> using busy
> waits in fireUntilHalt(). It should use monitors and
locks
> as it was
> doing before.
>
> Can anyone open a JIRA? also, if anyone would be
willing
> to take
> investigate and fix this one, it would really help.
> Otherwise, I will
> add it to my queue.
>
> Thanks,
>
> Edson
>
> 2010/10/25 Norman C <rent_my_time(a)yahoo.com>:
>
>
> Note that the 100% CPU issue with fireUntiHalt is only
with
> the 5.1
> version of
> Drools. In 5.0.x, CPU is close to 0% when there are
> no rules to fire. I
> saw
> this when I was testing upgrading to 5.1.
>
> Norman
>
>
>
> ----- Original Message ----
> From: jschmied <nabble(a)juergenschmied.de>
> To: rules-users(a)lists.jboss.org
> Sent: Sun, October 24, 2010 4:06:02 AM
> Subject: Re: [rules-users] Starting engine using
> fireUntilHalt and
> inserting no
> facts results in 50% CPU usage
>
>
> Hi!
>
> fireUntilHalt uses one processor with 100%. You have
a
> dualcore, so it's
> 50%. It's by design like this.
>
> You can:
> - Call fireAllRules after every insert if you have no
> ruleflow.
>
> - Use fireAllRules in a loop with a small sleep and
check
> for the end of
> the
> ruleflow in the loop:
>
> while (prc.getState() == ProcessInstance.STATE_ACTIVE)
{
> Threads.sleep(20);
> ksession.fireAllRules();
> }
>
> with both methods you get a low cpu load.
>
> bye
>
> juergen
> --
> View this message in context:
>
>
> http://drools-java-rules-engine.46999.n3.nabble.com/Starting-engine-using-fi
>
>
reUntilHalt-and-inserting-no-facts-results-in-50-CPU-usage-tp1760370p1761821
> .html
>
> Sent from the Drools - User mailing list archive at
> Nabble.com.
> _______________________________________________
> rules-users mailing list
> rules-users(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/rules-users
>
>
>
>
>
> _______________________________________________
> rules-users mailing list
> rules-users(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/rules-users
>
>
>
>
> --
> Edson Tirelli
> JBoss Drools Core Development
> JBoss by Red Hat @ www.jboss.com
>
> _______________________________________________
> rules-users mailing list
> rules-users(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/rules-users
>
>
> _______________________________________________
> rules-users mailing list
> rules-users(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/rules-users
>
>
>
>
> --
> Edson Tirelli
> JBoss Drools Core Development
> JBoss by Red Hat @ www.jboss.com
>
> _______________________________________________
> rules-users mailing list
> rules-users(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/rules-users
>
>
> _______________________________________________
> rules-users mailing list
> rules-users(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/rules-users
>
>
>
>
> --
> Edson Tirelli
> JBoss Drools Core Development
> JBoss by Red Hat @ www.jboss.com
>
> _______________________________________________
> rules-users mailing list
> rules-users(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/rules-users
>
>
>
>
> _______________________________________________
> rules-users mailing list
> rules-users(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/rules-users
>
10:22 a.m.
New subject: [rules-users] Starting engine using fireUntilHalt and inserting no facts results in 50% CPU usage
Hi Greg,
Thanks for the excellent analysis and the help. I agree with your findings.
For a solution though, I was thinking about just completely undoing
30163 (i.e., removing that missedNotifyAll flag as I think it just
complicates things) and simply adding a halt check inside the
synchronized block that calls the wait:
synchronized ( this.halt ) {
if( !this.halt.get() ) this.halt.wait();
}
Since the notify only happens on a synchronized block, I think that
this fixes the problem as it creates a happens-before relationship
between setting the halt flag and ( notifying all threads on the halt
monitor || checking the flag and executing the wait ).
Let me know what you think.
JIRA: https://jira.jboss.org/browse/JBRULES-2756
Changeset: http://fisheye.jboss.org/changelog/JBossRules/?cs=35744
Thanks,
Edson
2010/10/26 Greg Barton <greg_barton(a)yahoo.com>:
Here's a diff for the minimal change I think fixes the problem:
Index: src/main/java/org/drools/common/DefaultAgenda.java
===================================================================
--- src/main/java/org/drools/common/DefaultAgenda.java (revision 35727)
+++ src/main/java/org/drools/common/DefaultAgenda.java (working copy)
@@ -1048,10 +1048,10 @@
boolean fired = fireNextItem( agendaFilter );
fired = fired || !((AbstractWorkingMemory)
this.workingMemory).getActionQueue().isEmpty();
this.workingMemory.executeQueuedActions();
- if ( !fired && !missedNotifyAll.get()) {
+ if ( !fired ) {
try {
synchronized ( this.halt ) {
- this.halt.wait();
+ if(!missedNotifyAll.get()) this.halt.wait();
}
} catch ( InterruptedException e ) {
this.halt.set( true );
Basically, move the missedNotifyAll check immediately before the wait. This still avoids
the race condition, and only causes one spurious trip through the fireUntilHalt loop on
startup. I'd open a JIRA with this in it but I need to get to sleep. :)
--- On Tue, 10/26/10, Greg Barton <greg_barton(a)yahoo.com> wrote:
> From: Greg Barton <greg_barton(a)yahoo.com>
> Subject: Re: [rules-users] Starting engine using fireUntilHalt and inserting no facts
results in 50% CPU usage
> To: "Rules Users List" <rules-users(a)lists.jboss.org>,
rules-dev(a)lists.jboss.org
> Date: Tuesday, October 26, 2010, 1:01 AM
> The problem is the race condition fix
> introduced in rev 30163. The missedNotifyAll flag is
> set to true immediately which causes the busy loop.
> missedNotifyAll is initialized to false, but is immediately
> set to true because notifyHalt() is called in
> AbstractWorkingMemory.queueWorkingMemoryAction() via
> ReteooRuleBase.newStatefulSession(). Anyway, I'll move
> discussions of possible solutions over to the dev list,
> (crossposting now) or a JIRA if you'd prefer that.
>
> --- On Mon, 10/25/10, Greg Barton <greg_barton(a)yahoo.com>
> wrote:
>
> > From: Greg Barton <greg_barton(a)yahoo.com>
> > Subject: Re: [rules-users] Starting engine using
> fireUntilHalt and inserting no facts results in 50% CPU
> usage
> > To: "Rules Users List" <rules-users(a)lists.jboss.org>
> > Date: Monday, October 25, 2010, 3:18 PM
> > I was going to look at this at some
> > point. I guess that point will be tonight. :)
> >
> > GreG
> >
> > On Oct 25, 2010, at 15:02, Edson Tirelli <tirelli(a)post.com>
> > wrote:
> >
> > No worries, it is a good starting point anyway.
> > Hopefully someone
> > can continue from there or I will eventually fix it.
> Just a
> > lot of
> > stuff in my plate already that have priority over
> this.
> >
> > Edson
> >
> > 2010/10/25 mardo <mardo(a)abicola.de>:
> > Unfortunately no, I just had a look how it was
> implemented
> > (like
> > http://tutorials.jenkov.com/java-concurrency/thread-signaling.html#missedsig
> > nals) -> missed signals and noticed that it
> somehow
> > didn't work as intended.
> >
> > But no idea how to directly correct it, sorry.
> >
> > -----Original Message-----
> > From: rules-users-bounces(a)lists.jboss.org
> > [mailto:rules-users-bounces@lists.jboss.org]
> > On Behalf Of Edson Tirelli
> > Sent: Montag, 25. Oktober 2010 21:30
> > To: Rules Users List
> > Subject: Re: [rules-users] Starting engine using
> > fireUntilHalt and inserting
> > no facts results in 50% CPU usage
> >
> > Do you have a solution/patch for it?
> >
> > Edson
> >
> > 2010/10/25 mardo <mardo(a)abicola.de>:
> > Luckily just stumbled upon this mail...
> >
> > You can take a look at
> >
> > http://drools-java-rules-engine.46999.n3.nabble.com/fireUntilHalt-and-OSGi-C
> > PU-load-td1022352.html
> >
> > where I already provided a minimal example (don't know
> if
> > it covers all
> > cases) and tracked it down to missedNotifyAll in
> > DefaultAgenda
> >
> > cheers
> >
> >
> >
> > -----Original Message-----
> > From: rules-users-bounces(a)lists.jboss.org
> > [mailto:rules-users-bounces@lists.jboss.org]
> > On Behalf Of Edson Tirelli
> > Sent: Montag, 25. Oktober 2010 21:17
> > To: Rules Users List
> > Subject: Re: [rules-users] Starting engine using
> > fireUntilHalt and
> > inserting
> > no facts results in 50% CPU usage
> >
> > This is clearly a regression as Drools should be
> not be
> > using busy
> > waits in fireUntilHalt(). It should use monitors and
> locks
> > as it was
> > doing before.
> >
> > Can anyone open a JIRA? also, if anyone would be
> willing
> > to take
> > investigate and fix this one, it would really help.
> > Otherwise, I will
> > add it to my queue.
> >
> > Thanks,
> >
> > Edson
> >
> > 2010/10/25 Norman C <rent_my_time(a)yahoo.com>:
> >
> >
> > Note that the 100% CPU issue with fireUntiHalt is only
> with
> > the 5.1
> > version of
> > Drools. In 5.0.x, CPU is close to 0% when there are
> > no rules to fire. I
> > saw
> > this when I was testing upgrading to 5.1.
> >
> > Norman
> >
> >
> >
> > ----- Original Message ----
> > From: jschmied <nabble(a)juergenschmied.de>
> > To: rules-users(a)lists.jboss.org
> > Sent: Sun, October 24, 2010 4:06:02 AM
> > Subject: Re: [rules-users] Starting engine using
> > fireUntilHalt and
> > inserting no
> > facts results in 50% CPU usage
> >
> >
> > Hi!
> >
> > fireUntilHalt uses one processor with 100%. You have
> a
> > dualcore, so it's
> > 50%. It's by design like this.
> >
> > You can:
> > - Call fireAllRules after every insert if you have no
> > ruleflow.
> >
> > - Use fireAllRules in a loop with a small sleep and
> check
> > for the end of
> > the
> > ruleflow in the loop:
> >
> > while (prc.getState() == ProcessInstance.STATE_ACTIVE)
> {
> > Threads.sleep(20);
> > ksession.fireAllRules();
> > }
> >
> > with both methods you get a low cpu load.
> >
> > bye
> >
> > juergen
> > --
> > View this message in context:
> >
> >
> > http://drools-java-rules-engine.46999.n3.nabble.com/Starting-engine-using-fi
> >
> >
> reUntilHalt-and-inserting-no-facts-results-in-50-CPU-usage-tp1760370p1761821
> > .html
> >
> > Sent from the Drools - User mailing list archive at
> > Nabble.com.
> > _______________________________________________
> > rules-users mailing list
> > rules-users(a)lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/rules-users
> >
> >
> >
> >
> >
> > _______________________________________________
> > rules-users mailing list
> > rules-users(a)lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/rules-users
> >
> >
> >
> >
> > --
> > Edson Tirelli
> > JBoss Drools Core Development
> > JBoss by Red Hat @ www.jboss.com
> >
> > _______________________________________________
> > rules-users mailing list
> > rules-users(a)lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/rules-users
> >
> >
> > _______________________________________________
> > rules-users mailing list
> > rules-users(a)lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/rules-users
> >
> >
> >
> >
> > --
> > Edson Tirelli
> > JBoss Drools Core Development
> > JBoss by Red Hat @ www.jboss.com
> >
> > _______________________________________________
> > rules-users mailing list
> > rules-users(a)lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/rules-users
> >
> >
> > _______________________________________________
> > rules-users mailing list
> > rules-users(a)lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/rules-users
> >
> >
> >
> >
> > --
> > Edson Tirelli
> > JBoss Drools Core Development
> > JBoss by Red Hat @ www.jboss.com
> >
> > _______________________________________________
> > rules-users mailing list
> > rules-users(a)lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/rules-users
> >
> >
> >
> >
> > _______________________________________________
> > rules-users mailing list
> > rules-users(a)lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/rules-users
> >
>
>
>
>
_______________________________________________
rules-dev mailing list
rules-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/rules-dev
--
Edson Tirelli
JBoss Drools Core Development
JBoss by Red Hat @ www.jboss.com
5141
days inactive
5141
days old
1 comments
2 participants
participants (2)
-
Edson Tirelli -
Greg Barton