netbeans icon indicating copy to clipboard operation
netbeans copied to clipboard

GlassFish modules cleaning and some fixes

Open pepness opened this issue 2 years ago • 1 comments

NetBeans GlassFish module notes:

  • Use ConcurrentHashMap insted of Collections.synchronizedMap
  • Use ConcurrentHashMap.newKeySet instead of Collections.synchronizedMap
  • When using ConcurrentHashMap remove some synchronized blocks when using atomic and concurrent operations (remove, put, containsKey, and get).
  • Refactor to computeIfAbsent pattern when possible.
  • Add javadoc support for JAKARTA EE 9 and JAKARTA EE 10
  • Add missing validations for Jakarta EE 10
  • Close resource with try-with-resources
  • Remove redundant calls to si.getDeployerUri() and this.getClass()
  • Remove redundant call to some methods getConfigRoot(), getDeployerUri()
  • Use GlassFishLogger instead of generic glassfish-ee logger
  • Use target class for logger instead of "glassfish"
  • Regen sig file because ant check-sigtests-release failed
  • Fix path to javadocs when creating a new ConfigBuilder
  • Make final some class instances
  • Refactor method versionToResourceFilesIndexes()
  • Fix some javadoc comments
  • Make final some class instances
  • Add imports and remove commented code
  • Add TODO comments
  • Change inefficient creation of new Array
  • Remove commented code
  • Fix some typos

NetBeans Testing:

  • Verify successful execution of libraries and licenses Ant test
  • Verify successful execution of Verify Sigtests
  • Verify successful execution of ant -Dcluster.config=release commit-validation
  • Verify successful execution of unit tests for modules glassfish.common, glassfish.javaee, glassfish.tooling, and glassfish.eecommon
  • Started NetBeans and ensure the log didn't have any ERROR or new WARNINGS
  • Successfully register GlassFish versions 4, 5, 5.1, 6, 6.2, and 7. Then create a web app for each version and verify that they work.

pepness avatar Oct 19 '23 23:10 pepness

what anticipated performance issue is the rewrite in CHM supposed to improve? The access pattern looks like there is probably not much contention happening. Once the instances are in the map, the read operations should be really fast while using synchronization since the critical sections are tiny.

the refactoring does also not get rid of many critical sections, e.g:

        public boolean isEmpty() {
            synchronized(delegate) {return delegate.isEmpty();}
        }

so there would be more work left to finish it.

I usually would see concurrency updates a bit outside the scope of cleanups. Is it worth spending the extra time in review for risky changes when the access pattern is ending up to be get() after short initialization?

mbien avatar Oct 28 '23 17:10 mbien