glassfish
  1. glassfish
  2. GLASSFISH-15784

[PERF] ConnectorObjectFactor.getObjectInstance performance has regressed

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.1_b41
    • Component/s: jca
    • Labels:
      None

      Description

      Performance of ConnectorObjectFactor.getObjectInstance has regressed in V3.1 due to the section of code added for dynamic reconfiguration. In particular, continually calling resources.getResourceByName(ResourcePool.class, poolInfo.getName()) is a performance drain because it depends on expensive JNDI lookups.

        Activity

        Hide
        Jagadish added a comment - - edited

        Refer the issues
        http://java.net/jira/browse/GLASSFISH-14454
        http://java.net/jira/browse/GLASSFISH-14731
        for more background w.r.t config bean calls (resources.getResources() or resources.getResourceByName() or resource.getEnabled() ) related performance issues.

        Config bean calls seem to be expensive.

        I am working on a fix which will avoid the reported config-bean call in the above block of code by default. Only when the pool is enabled with transparent dynamic reconfiguration flag, config-bean related calls will be made.

        Following might be a more appropriate (complete solution) solution which should be evaluated for 3.2 :

        Eventually, we need a fix in Config layer to make these calls (atleast the read-only calls) less expensive so that every module benefits from it.
        or
        We may have to back the config bean for all resources and pools

        Show
        Jagadish added a comment - - edited Refer the issues http://java.net/jira/browse/GLASSFISH-14454 http://java.net/jira/browse/GLASSFISH-14731 for more background w.r.t config bean calls (resources.getResources() or resources.getResourceByName() or resource.getEnabled() ) related performance issues. Config bean calls seem to be expensive. I am working on a fix which will avoid the reported config-bean call in the above block of code by default. Only when the pool is enabled with transparent dynamic reconfiguration flag, config-bean related calls will be made. Following might be a more appropriate (complete solution) solution which should be evaluated for 3.2 : Eventually, we need a fix in Config layer to make these calls (atleast the read-only calls) less expensive so that every module benefits from it. or We may have to back the config bean for all resources and pools
        Hide
        Jagadish added a comment -

        How bad is its impact? (Severity)

        • Affects Performance tests
          Identify why the fix needs to occur now:
        • Is a regression of performance available in a prior release

        How often does it happen? (Frequency)

        • Every time application tries to acquire a "datasource".

        How much effort is required to fix it? (Cost)

        • Minor, fix is to avoid this expensive call by caching the attribute.

        What is the risk of fixing it? (Risk)

        • Minor, made sure that all of the tests pass (QL (Web, Classic), connector-dev, jdbc-dev, connector-standalone-cts (web, classic), resources-admin-cli, connector-sqe)

        Does a work around for the issue exist? Can the workaround be reasonably employed by the end user?

        • No, as it affects performance tests.

        If the issue is not fixed should the issue and its workaround (if applicable) be described in the Release Notes?

        • NA

        How long has the bug existed in the product?

        • Performance team has revealed it today.

        Do regression tests exist for this issue?

        • Since this is a performance issue, no feature is being tested here.
        • Also, sent the patch to Scott, Amit for verification.
        • Made sure that the reported call does not happen by default.

        Which tests should QA (re)run to verify the fix did not destabilize GlassFish?

        • NA

        When will a tested fix be ready for integration?

        • Fix is available, waiting for Scott/Amits confirmation.
        Show
        Jagadish added a comment - How bad is its impact? (Severity) Affects Performance tests Identify why the fix needs to occur now: Is a regression of performance available in a prior release How often does it happen? (Frequency) Every time application tries to acquire a "datasource". How much effort is required to fix it? (Cost) Minor, fix is to avoid this expensive call by caching the attribute. What is the risk of fixing it? (Risk) Minor, made sure that all of the tests pass (QL (Web, Classic), connector-dev, jdbc-dev, connector-standalone-cts (web, classic), resources-admin-cli, connector-sqe) Does a work around for the issue exist? Can the workaround be reasonably employed by the end user? No, as it affects performance tests. If the issue is not fixed should the issue and its workaround (if applicable) be described in the Release Notes? NA How long has the bug existed in the product? Performance team has revealed it today. Do regression tests exist for this issue? Since this is a performance issue, no feature is being tested here. Also, sent the patch to Scott, Amit for verification. Made sure that the reported call does not happen by default. Which tests should QA (re)run to verify the fix did not destabilize GlassFish? NA When will a tested fix be ready for integration? Fix is available, waiting for Scott/Amits confirmation.
        Hide
        Nazrul added a comment -

        Approved for checkin pending confirmation from Performance team. Please run the Dev Tests to make sure that this is not causing any regression. Also, please do a code review before committing the changes.

        Show
        Nazrul added a comment - Approved for checkin pending confirmation from Performance team. Please run the Dev Tests to make sure that this is not causing any regression. Also, please do a code review before committing the changes.
        Hide
        Scott Oaks added a comment -

        Perf tests show a 1% improvement with the patch.

        Show
        Scott Oaks added a comment - Perf tests show a 1% improvement with the patch.
        Hide
        Jagadish added a comment -

        Made sure that all of the tests pass (QL (Web, Classic), connector-dev, jdbc-dev, connector-standalone-cts (web, classic), resources-admin-cli, connector-sqe)

        Fixed in trunk:
        svn log -v -r 44853
        Modified Paths:
        ---------------
        trunk/v3/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/deployer/JdbcConnectionPoolDeployer.java
        trunk/v3/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/naming/ConnectorObjectFactory.java
        trunk/v3/connectors/connectors-runtime/src/main/java/com/sun/enterprise/connectors/ConnectorRegistry.java

        Fixed in 3.1 branch :
        svn log -v -r 44847
        Modified Paths:
        ---------------
        branches/3.1/connectors/connectors-runtime/src/main/java/com/sun/enterprise/connectors/ConnectorRegistry.java
        branches/3.1/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/naming/ConnectorObjectFactory.java
        branches/3.1/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/deployer/JdbcConnectionPoolDeployer.java

        Show
        Jagadish added a comment - Made sure that all of the tests pass (QL (Web, Classic), connector-dev, jdbc-dev, connector-standalone-cts (web, classic), resources-admin-cli, connector-sqe) Fixed in trunk: svn log -v -r 44853 Modified Paths: --------------- trunk/v3/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/deployer/JdbcConnectionPoolDeployer.java trunk/v3/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/naming/ConnectorObjectFactory.java trunk/v3/connectors/connectors-runtime/src/main/java/com/sun/enterprise/connectors/ConnectorRegistry.java Fixed in 3.1 branch : svn log -v -r 44847 Modified Paths: --------------- branches/3.1/connectors/connectors-runtime/src/main/java/com/sun/enterprise/connectors/ConnectorRegistry.java branches/3.1/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/naming/ConnectorObjectFactory.java branches/3.1/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/deployer/JdbcConnectionPoolDeployer.java

          People

          • Assignee:
            Jagadish
            Reporter:
            Scott Oaks
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: