> On March 9, 2017, 6:27 p.m., Kirk Lund wrote: > > Here's the change I recommend for improving performance. The problem is > > this line... > > > > private static SecurityService defaultInstance = new > > IntegratedSecurityService(); > > > > It always installs a IntegratedSecurityService. Branch here and install > > IntegratedSecurityService iif we have security enabled; else we install > > NullSecurityService which is a no-op that does nothing but return false, > > etc. > > Jinmei Liao wrote: > but we need the object to determine whether it is enabled or not. With it > being null, how can we determine if we have security enabled or not? > > Kirk Lund wrote: > It's not null, the name is NullSecurityService because it follows the > Null Object Pattern. Imagine if you call it NoOpSecurityService or > NoSecurityService. > > Kirk Lund wrote: > And it requires further refactoring. The static "getSecurityService" > would need to move to a different class. > > SecurityService securityService = SecurityService.getSecurityService(); > > If Security is enabled then the above returns an instance of > IntegratedSecurityService. > > If Security is disabled then the above returns an instance of > NullSecurityService. All of the methods return false, null or if return type > is void then the method is simply empty. JIT will eventually remove the calls > to methods that are empty so runtime gets even faster.
Oh, I see what you mean. As it requires more refactoring (pulling the code to initSecurity, and check for isIntegratedSecurity out of the interface), can we do this for now, and leave the ticket open for this further improvement? - Jinmei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57466/#review168485 ----------------------------------------------------------- On March 9, 2017, 3:52 p.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57466/ > ----------------------------------------------------------- > > (Updated March 9, 2017, 3:52 p.m.) > > > Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk > Lund. > > > Repository: geode > > > Description > ------- > > * clean up interface, reduce function call > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java > 8366930a3bf6a244d077c745fc810d77c4699c38 > > geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java > 14784c391212095413c0d577cfc65de7247080b5 > > geode-core/src/main/java/org/apache/geode/management/internal/security/AccessControlMBean.java > 6514a33e52611994ddc16a58414146ebaad75c65 > geode-core/src/main/java/org/apache/geode/security/ResourcePermission.java > 45da464419779773c9116d824fcf11774bafbd79 > > > Diff: https://reviews.apache.org/r/57466/diff/1/ > > > Testing > ------- > > precheckin successful > > > Thanks, > > Jinmei Liao > >
