Skip to content

Commit f079dcd

Browse files
committed
perf: eliminate hot-path filesystem and regex costs in handler/routing layer
Four targeted optimizations on the per-request execution path: 1. HandlerService: cache ImplicitViews setting `getSetting("ImplicitViews")` was called on every handler dispatch where the requested action was missing. It is now cached to variables scope in onConfigurationLoad() alongside every other already-cached setting. 2. HandlerService.isViewDispatch: drop redundant fileExists(expandPath()) locateView/locateModuleView already verify file existence and return the path with a .cfm/.bxm extension only when the file is confirmed present. The follow-up fileExists() call was re-doing answered work. Replaced with a cheap string suffix check — zero filesystem I/O. 3. RoutingService.findRoute: use cached canDebug local variable consistently canDebug was already stored in a local variable at the top of findRoute() but three spots inside the same method re-called getLogger().canDebug() instead of using it. All three now use the cached local variable. 4. Router/RoutingService: pre-parse response string placeholders at registration renderResponse() was running reMatchNoCase + reReplaceNoCase on the route's static response string on every request. Tokens are now parsed once in addRoute() into a responsePlaceholders array; renderResponse() iterates the pre-parsed list with no regex at runtime. Also fixes a latent bug where multiple placeholders were not all applied (original always replaced into the original string rather than the accumulated result). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MUinjJLvJod1u9s2e9T6zc
1 parent d6ccd51 commit f079dcd

5 files changed

Lines changed: 102 additions & 14 deletions

File tree

system/web/routing/Router.cfc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,6 +1065,13 @@ component
10651065
}
10661066
}
10671067

1068+
// Pre-parse response string placeholders so renderResponse() skips regex on every request
1069+
if ( isSimpleValue( thisRoute.response ) && len( thisRoute.response ) ) {
1070+
thisRoute.responsePlaceholders = reMatchNoCase( "{[^{]+?}", thisRoute.response ).map( function( token ){
1071+
return { token : token, key : reReplaceNoCase( token, "({|})", "", "all" ) };
1072+
} );
1073+
}
1074+
10681075
// Add it to the corresponding routing table
10691076
// MODULES
10701077
if ( len( arguments.module ) ) {
@@ -1133,6 +1140,7 @@ component
11331140
"rc" : {}, // The RC params to add incorporate if matched
11341141
"redirect" : "", // The redirection location
11351142
"response" : "", // Do we have an inline response closure
1143+
"responsePlaceholders" : [], // Pre-parsed {token} list for string responses
11361144
"ssl" : false, // Are we forcing SSL
11371145
"statusCode" : 200, // The response status code
11381146
"valuePairTranslation" : true, // If we translate name-value pairs in the URL by convention

system/web/services/HandlerService.cfc

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" {
8282
variables.handlersPath = variables.controller.getSetting( "handlersPath" )
8383
variables.interceptorService = variables.controller.getInterceptorService()
8484
variables.invalidEventHandler = variables.controller.getSetting( "invalidEventHandler" )
85+
variables.implicitViews = variables.controller.getSetting( "ImplicitViews" )
8586
variables.modules = variables.controller.getSetting( "modules" )
8687
variables.templateCache = variables.controller.getCache( "template" )
8788
variables.wirebox = variables.controller.getWireBox()
@@ -155,7 +156,7 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" {
155156

156157
// Test for Implicit View Dispatch
157158
if (
158-
controller.getSetting( "ImplicitViews" ) AND
159+
variables.implicitViews AND
159160
isViewDispatch( arguments.ehBean.getFullEvent(), arguments.ehBean )
160161
) {
161162
return oEventHandler;
@@ -404,8 +405,9 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" {
404405
targetView = renderer.locateView( cEvent );
405406
}
406407

407-
// CFML View
408-
if ( fileExists( expandPath( targetView ) ) ) {
408+
// locateView/locateModuleView return a path with .cfm/.bxm extension only when
409+
// the file was verified to exist — no need for a redundant filesystem call here.
410+
if ( right( targetView, 4 ) == ".cfm" || right( targetView, 4 ) == ".bxm" ) {
409411
arguments.ehBean.setViewDispatch( true );
410412
return true;
411413
}

system/web/services/RoutingService.cfc

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,7 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" {
533533

534534
// Check if we found a route, else just return empty params struct
535535
if ( results.route.isEmpty() ) {
536-
if ( getLogger().canDebug() ) {
536+
if ( canDebug ) {
537537
getLogger().debug( "No URL routes matched on routed string: #requestString#" );
538538
}
539539
return results;
@@ -599,7 +599,7 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" {
599599
// reset pattern matching, if packages found.
600600
if ( compare( packagedRequestString, requestString ) NEQ 0 ) {
601601
// Log package resolved
602-
if ( getLogger().canDebug() ) {
602+
if ( canDebug ) {
603603
getLogger().debug( "URL Routing Package Resolved: #packagedRequestString#" );
604604
}
605605
// Return found Route recursively.
@@ -781,16 +781,14 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" {
781781
// simple values
782782
if ( isSimpleValue( aRoute.response ) ) {
783783
// setup default response
784-
theResponse = aRoute.response;
785-
// String replacements
786-
var replacements = reMatchNoCase( "{[^{]+?}", aRoute.response );
787-
for ( var thisReplacement in replacements ) {
788-
var thisKey = reReplaceNoCase( thisReplacement, "({|})", "", "all" );
789-
if ( event.valueExists( thisKey ) ) {
784+
theResponse = aRoute.response;
785+
// Apply pre-parsed placeholders (tokens resolved once at route registration)
786+
for ( var placeholder in aRoute.responsePlaceholders ) {
787+
if ( event.valueExists( placeholder.key ) ) {
790788
theResponse = replace(
791-
aRoute.response,
792-
thisReplacement,
793-
event.getValue( thisKey ),
789+
theResponse,
790+
placeholder.token,
791+
event.getValue( placeholder.key ),
794792
"all"
795793
);
796794
}

tests/specs/web/routing/RouterTest.cfc

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,61 @@ component extends="coldbox.system.testing.BaseModelTest" {
666666
} );
667667
} );
668668
} );
669+
670+
describe( "Response placeholder pre-parsing", function(){
671+
beforeEach( function( currentSpec ){
672+
router = createMock( "coldbox.system.web.routing.Router" )
673+
.init()
674+
.setController( controller )
675+
.setLogBox( controller.getLogBox() )
676+
.setLog( controller.getLogBox().getLogger( this ) )
677+
.setCacheBox( controller.getCacheBox() )
678+
.setWireBox( controller.getWireBox() );
679+
} );
680+
681+
it( "a route with a static string response pre-parses placeholders at registration", function(){
682+
router.addRoute( pattern = "/hello/:name", response = "Hello {name}!" );
683+
var routes = router.getRoutes();
684+
expect( routes ).toHaveLength( 1 );
685+
expect( routes[ 1 ] ).toHaveKey( "responsePlaceholders" );
686+
expect( routes[ 1 ].responsePlaceholders ).toHaveLength( 1 );
687+
expect( routes[ 1 ].responsePlaceholders[ 1 ].token ).toBe( "{name}" );
688+
expect( routes[ 1 ].responsePlaceholders[ 1 ].key ).toBe( "name" );
689+
} );
690+
691+
it( "a route with multiple placeholders pre-parses all of them", function(){
692+
router.addRoute( pattern = "/greet/:name/:mod", response = "Hello {name} from {mod}" );
693+
var routes = router.getRoutes();
694+
expect( routes[ 1 ].responsePlaceholders ).toHaveLength( 2 );
695+
expect( routes[ 1 ].responsePlaceholders[ 1 ].key ).toBe( "name" );
696+
expect( routes[ 1 ].responsePlaceholders[ 2 ].key ).toBe( "mod" );
697+
} );
698+
699+
it( "a route with no placeholders has an empty responsePlaceholders array", function(){
700+
router.addRoute( pattern = "/static", response = "No placeholders here" );
701+
var routes = router.getRoutes();
702+
expect( routes[ 1 ].responsePlaceholders ).toBeArray();
703+
expect( routes[ 1 ].responsePlaceholders ).toHaveLength( 0 );
704+
} );
705+
706+
it( "a route with a closure response has an empty responsePlaceholders array", function(){
707+
router.addRoute(
708+
pattern = "/closure",
709+
response = function( event, rc, prc ){ return "hi"; }
710+
);
711+
var routes = router.getRoutes();
712+
expect( routes[ 1 ].responsePlaceholders ).toBeArray();
713+
expect( routes[ 1 ].responsePlaceholders ).toHaveLength( 0 );
714+
} );
715+
716+
it( "a route with no response has an empty responsePlaceholders array", function(){
717+
router.addRoute( pattern = "/noop", event = "main.index" );
718+
var routes = router.getRoutes();
719+
expect( routes[ 1 ].responsePlaceholders ).toBeArray();
720+
expect( routes[ 1 ].responsePlaceholders ).toHaveLength( 0 );
721+
} );
722+
} );
723+
} );
669724
}
670725

671726
}

tests/specs/web/services/HandlerServiceTest.cfc

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,31 @@ component extends="tests.resources.BaseIntegrationTest" {
194194
} );
195195
} );
196196
} );
197+
198+
describe( "Hot-path caching optimizations", () => {
199+
beforeEach( () => {
200+
setup();
201+
variables.handlerService = controller.getHandlerService();
202+
} );
203+
204+
it( "caches implicitViews setting in variables scope after configuration load", () => {
205+
// implicitViews must be a boolean cached from getSetting("ImplicitViews")
206+
expect( variables.handlerService.$getProperty( "implicitViews", "variables" ) ).toBeBoolean();
207+
expect( variables.handlerService.$getProperty( "implicitViews", "variables" ) ).toBeTrue();
208+
} );
209+
210+
it( "isViewDispatch detects a view by extension without a filesystem call", () => {
211+
// simpleview exists — getHandlerBean triggers isViewDispatch internally
212+
// If the extension-check logic is wrong the viewDispatch flag won't be set
213+
var results = variables.handlerService.getHandlerBean( "simpleview" );
214+
expect( results.getViewDispatch() ).toBeTrue();
215+
} );
216+
217+
it( "isViewDispatch returns false for an event with no matching view", () => {
218+
var results = variables.handlerService.getHandlerBean( "nonexistent_view_xyz" );
219+
expect( results.getViewDispatch() ).toBeFalse();
220+
} );
221+
} );
197222
}
198223

199224
}

0 commit comments

Comments
 (0)