groovy

GroovyScriptEngineImpl could use ConcurrentHashMaps

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.8.5, 2.0-beta-2
  • Fix Version/s: 1.8.6, 2.0-beta-3
  • Component/s: None
  • Number of attachments :
    1

Description

The Groovy implementation of JSR-223 (Scripting for the Java Platform), GroovyScriptEngineImpl, uses Collections.synchronizedMap(new HashMap()) for the caches of compiled scripts and closures. Since JSR-223 was first supported in Java 6, it's safe to instead use ConcurrentHashMap for those data structures. According to JCIP (Java Concurrency in Practice), ConcurrentHashMap performs substantially better than Hashtable or Collections.synchronizedMap(). In addition, once ConcurrentHashMaps are used, the code could take advantage of additional methods provided in the ConcurrentMap interface, such as putIfAbsent().

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Matt Passell added a comment -

This file patches GroovyScriptEngineImpl to use ConcurrentHashMaps.

Show
Matt Passell added a comment - This file patches GroovyScriptEngineImpl to use ConcurrentHashMaps.
Matt Passell made changes -
Field Original Value New Value
Attachment GROOVY-5241.patch [ 58418 ]
Cedric Champeau made changes -
Status Open [ 1 ] Resolved [ 5 ]
Assignee Cedric Champeau [ melix ]
Fix Version/s 2.0-beta-3 [ 18244 ]
Fix Version/s 1.8.6 [ 18245 ]
Resolution Fixed [ 1 ]
Hide
Permalink
Jim White added a comment -

Please do not make this change until Groovy no longer supports JDK 1.5. JSR-223 is fully supported for JDK 1.5 which is why we use the LiveTribe JSR-223 implementation for building (and which users can deploy if their runtime is JDK 1.5 rather than JDK 1.6+).

Show
Jim White added a comment - Please do not make this change until Groovy no longer supports JDK 1.5. JSR-223 is fully supported for JDK 1.5 which is why we use the LiveTribe JSR-223 implementation for building (and which users can deploy if their runtime is JDK 1.5 rather than JDK 1.6+).
Hide
Permalink
Jim White added a comment -

This should not be done until Groovy no longer supports JDK 1.5.

Show
Jim White added a comment - This should not be done until Groovy no longer supports JDK 1.5.
Jim White made changes -
Resolution Fixed [ 1 ]
Status Resolved [ 5 ] Reopened [ 4 ]
Hide
Permalink
Matt Passell added a comment -

ConcurrentHashMap was introduced in Java 5 (aka JDK 1.5), so there's no issue - see http://docs.oracle.com/javase/1.5.0/docs/api/java/util/concurrent/ConcurrentHashMap.html

Show
Matt Passell added a comment - ConcurrentHashMap was introduced in Java 5 (aka JDK 1.5), so there's no issue - see http://docs.oracle.com/javase/1.5.0/docs/api/java/util/concurrent/ConcurrentHashMap.html
Hide
Permalink
Jim White added a comment -

That's fine if that's the case. I was reacting to the justification you gave which was that JDK 1.6 would be in use if JSR-223 were.

Show
Jim White added a comment - That's fine if that's the case. I was reacting to the justification you gave which was that JDK 1.6 would be in use if JSR-223 were.
Hide
Permalink
Cedric Champeau added a comment -

Yes, ConcurrentHashMap is there since 1.5, there's no problem here.

Show
Cedric Champeau added a comment - Yes, ConcurrentHashMap is there since 1.5, there's no problem here.
Cedric Champeau made changes -
Status Reopened [ 4 ] Resolved [ 5 ]
Resolution Fixed [ 1 ]
Hide
Permalink
Matt Passell added a comment - - edited

Sorry for the confusion, Jim. I had no idea that JSR-223 had been backported. When I wrote up the description, I should have just said something like, "since Groovy 1.8+ runs in a JDK 1.5+ environment, it's safe to use ConcurrentHashMap..."

Show
Matt Passell added a comment - - edited Sorry for the confusion, Jim. I had no idea that JSR-223 had been backported. When I wrote up the description, I should have just said something like, "since Groovy 1.8+ runs in a JDK 1.5+ environment, it's safe to use ConcurrentHashMap..."
Paul King made changes -
Status Resolved [ 5 ] Closed [ 6 ]

People

  • Assignee:
    Cedric Champeau
    Reporter:
    Matt Passell
Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: