History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: DROOLS-446
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: Critical Critical
Assignee: Michael Neale
Reporter: Jerry Cattell
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
drools-legacy

Performance degradation due to PrimitiveLongMap

Created: 17/Nov/05 03:03 PM   Updated: 14/Dec/05 02:32 AM  Due: 22/Nov/05
Component/s: reteoo
Affects Version/s: 2.0-final, 2.1
Fix Version/s: 2.5

Time Tracking:
Not Specified

File Attachments: 1. Java Source File DroolsPerformanceExample.java (2 kb)
2. File odds.java.drl (0.7 kb)
3. Java Source File WorkingMemoryImpl.java (20 kb)

Environment: JDK 1.4.2_08


 Description  « Hide
I am experiencing performance problems using drools with a very simple use case. I have 50-100 rules, 5-10 facts, and assert facts and fire the rules hundreds to thousands of times per second. I have attached a basic example of the problem I am encountering. You'll see that as it loops, creating new WorkingMemory objects, asserting facts, and firing rules, the time to do this takes longer and longer until it gets to an unusable state.

After running this through JProfiler, I found the hot spot to be PrimitiveLongMap. When the RuleBase is cached and new WorkingMemoryImpls are created (as the preferred approach seems to be), the generated FactHandle ids continue to increase. The problem is that as the number gets large, say over 1 million, the PrimitiveLongMap seems to spend all of its time creating Pages, or alternatively, iterating through all of the pages to return the collection of values when WorkingMemoryImpl.getObjects is called.

I was able to fix this for my use case by simply replacing PrimitiveLongMap with HashMap. Of course, I'm assuming PrimitiveLongMap was used to replace a HashMap for a reason (very large number of facts? Long object creation?), so this is probably not a long term solution for the drools project. So I see three other options:

1. Fix PrimitiveLongMap. I haven't spent the time to try and understand what type of work this would require.

2. Replace PrimitiveLongMap with a different implementation (perhaps one of the ones listed on http://blogs.codehaus.org/people/mproctor/archives/000890_primitive_map_implementation_and_benchmark_comparisons.html ).

3. Generate fact handle ids on a per working memory basis (as shown in the WorkingMemoryImpl.java included). I don't know if the RuleBase ever uses the FactHandle id in a global way or always just passes the FactHandle object through to a WorkingMemoryImpl, so that could lead to potential problems.

Let me know if any additional details are required.



 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
Mark Proctor - 19/Nov/05 04:29 AM
HashMaps kill performance, PrimitiveLongMap is used to increase performance and greatly reduce memory consumption for the Drools use case (linear increasing fact ids); see http://blogs.codehaus.org/people/mproctor/archives/000890_primitive_map_implementation_and_benchmark_comparisons.html

Drools recycles fact handle ids, we have a PrimitiveLongStack for exactly this purpose - but of course you have to release ids for them to be recycled...

Ids are unique to a WM so no reason at all to not create a handle factory per WM - which is a light weight operation anyway. Seems with a new factory per WM and the existing ids recycler we should solve all Drools problems, i.e. a 5 minute coding operation to fix this

However long term I would like to see PrimitiveLongMap become more useful for other projects. I think this can be achieved with another paging system ontop of the existing page layout that manages the colllapsing of pages - would be an interesting and fun task

Mark


Michael Neale - 27/Nov/05 10:08 PM
Have a fix working. Am refactoring APIs to not expose the factory other then in the Impl classes (as they shouldn't be exposed). Called to RuleBase.newWorkingMemory() will be idempotent (as they should be) on the RuleBase.

Michael Neale - 14/Dec/05 02:32 AM
done