Bug 56582 - Use switch(enum) in implementations of ActionHook.action(ActionCode, ...)
Use switch(enum) in implementations of ActionHook.action(ActionCode, ...)
Status: RESOLVED FIXED
Product: Tomcat 8
Classification: Unclassified
Component: Connectors
8.0.8
All All
: P2 enhancement (vote)
: ----
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2014-06-01 23:59 UTC by Konstantin Kolinko
Modified: 2014-06-05 00:56 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Konstantin Kolinko 2014-06-01 23:59:27 UTC
As of now,
AbstractHttp11Processor.action(ActionCode, Object)
AbstractAjpProcessor.action(ActionCode, Object)

are implemented as a chain of if/elseif going through every defined ActionCode.

As ActionCode is an enum starting with Tomcat 7, I think it would be better to reorganize the code to use switch(enum) construct.
Comment 1 Konstantin Kolinko 2014-06-03 00:22:32 UTC
Done for Tomcat 8 (r1599385 r1599393 r1599395), will be in 8.0.9.

I plan to backport this change to Tomcat 7, as I think switch() behaves better for performance.

One possible error spotted thanks to IDE warning:
DISPATCH_EXECUTE action code is not processed by AbstractAjpProcessor.
I placed a FIXME comment there.

The fix is likely to copy the code from AbstractHttp11Processor.action():
        case DISPATCH_EXECUTE: {
            getEndpoint().executeNonBlockingDispatches(socketWrapper);
            break;
        }
Comment 2 Konstantin Kolinko 2014-06-03 18:16:00 UTC
Done for Tomcat 7 (r1599505 r1599711) and will be in 7.0.55.

I am ready to close this issue, but  DISPATCH_EXECUTE + AJP question for Tomcat 8 from Comment 1 is pending.
Comment 3 Konstantin Kolinko 2014-06-03 19:20:23 UTC
(In reply to Konstantin Kolinko from comment #1)

Implemented DISPATCH_EXECUTE action code for AJP connectors in Tomcat 8 by r1599752, will be in 8.0.9.
Comment 4 Christopher Schultz 2014-06-05 00:56:19 UTC
(In reply to Konstantin Kolinko from comment #1)
> I plan to backport this change to Tomcat 7, as I think switch() behaves
> better for performance.

+1

A tableswitch will be faster than a series of comparisons and jumps. If the set of cases is sufficiently sparse, it will degrade to a lookupswitch which is no slower than the old if/elseif implementation and may be a bit faster as lookupswitch is likely to be a highly-optimized JVM operation not requiring any JIT trickery.