Yep. :) I always propose minimal change solutions first, but having the single flag is
best in this case.
GreG
On Oct 26, 2010, at 10:22, Edson Tirelli <tirelli(a)post.com> wrote:
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
_______________________________________________
rules-dev mailing list
rules-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/rules-dev