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 <[email protected]> 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 <[email protected]>: 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 <[email protected]> wrote: From: Greg Barton <[email protected]> Subject: Re: [rules-users] Starting engine using fireUntilHalt and inserting no facts results in 50% CPU usage To: "Rules Users List" <[email protected]>, [email protected] 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 <[email protected]> wrote: From: Greg Barton <[email protected]> Subject: Re: [rules-users] Starting engine using fireUntilHalt and inserting no facts results in 50% CPU usage To: "Rules Users List" <[email protected]> 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 <[email protected]> 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 <[email protected]>: 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: [email protected] [mailto:[email protected]] 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 <[email protected]>: 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: [email protected] [mailto:[email protected]] 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 <[email protected]>: 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 <[email protected]> To: [email protected] 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 [email protected] https://lists.jboss.org/mailman/listinfo/rules-users _______________________________________________ rules-users mailing list [email protected] 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 [email protected] https://lists.jboss.org/mailman/listinfo/rules-users _______________________________________________ rules-users mailing list [email protected] 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 [email protected] https://lists.jboss.org/mailman/listinfo/rules-users _______________________________________________ rules-users mailing list [email protected] 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 [email protected] https://lists.jboss.org/mailman/listinfo/rules-users _______________________________________________ rules-users mailing list [email protected] https://lists.jboss.org/mailman/listinfo/rules-users _______________________________________________ rules-dev mailing list [email protected] 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 [email protected] https://lists.jboss.org/mailman/listinfo/rules-dev _______________________________________________ rules-dev mailing list [email protected] https://lists.jboss.org/mailman/listinfo/rules-dev
