Closed Bug 383592 Opened 17 years ago Closed 5 years ago

Switch to HashMap for ScriptableObject property storage

Categories

(Rhino Graveyard :: Core, defect)

1.6R6
x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INACTIVE

People

(Reporter: norrisboyd, Unassigned)

Details

In bug 360964 it was proposed that we switch to HashMap for Scriptable object property storage, and maybe even implement some sort of pluggable architecture.
Yeah, a pluggable architecture would be in order, in particular to give people using objects from multiple threads more control -- I can imagine some people would want a simple hashmap, others a synchronized one, yet others could use a java.util.ConcurrentHashMap. Of course, this also might go into the issues of explicit thread-safety measures within the ScriptableObject...
#412928 contains a patch providing the needed refactoring.
I worked on this a while, trying various optimizations, but I couldn't find a way to switch to standard Map classes without a significant performance penalty. 

I believe the main need for this change was to maintain the insertion order of properties for iteration (see bug 419090), so I've just checked in a change that accomplishes this by modifying the current Rhino ScriptableObject implementation rather than replacing it. This change has minimal performance impact.

So I'm thinking I won't fix this bug, at least anytime soon. There's some value in a pluggable Map implementation, but the runtime cost is surprisingly high.
Note that ScriptableObject now implements java.util.Map (see bug 448816), which might be of interest to some people looking at this bug.

Closing. Bug management is now done here:
https://github.com/mozilla/rhino

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → INACTIVE
You need to log in before you can comment on or make changes to this bug.