Skip to content

Commit 8f73b77

Browse files
bpamiriclaude
andauthored
perf(model): cache mixin-integration plan to fix per-instance overhead (#3236)
Cache the mixin-integration plan (directory scan + per-file createObject + getMetaData, plus pre-resolved public-method references and a precomputed mixin-override set) once per application instead of re-paying it on every model/controller/mapper materialization. Fixes the per-instance overhead behind #3213; semantics unchanged. Includes an engine-portable guard spec. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0167uSbSN4vZqQqL5QZfdiQm
1 parent d0c343a commit 8f73b77

7 files changed

Lines changed: 296 additions & 130 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Model, controller, and mapper object creation no longer re-scans the framework mixin folders (a directory listing plus a `createObject` and `getMetaData` per file) on every materialization. The mixin-integration plan is now built once per application and reused, and the per-method `$willBeOverriddenByMixin` lookup is precomputed — cutting model-instance creation roughly in half (every `new()` and every finder row was paying the full cost). This is the regression behind slow test-suite and request times reported on 4.0.x (#3213)

vendor/wheels/Controller.cfc

Lines changed: 30 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -375,69 +375,46 @@ component output="false" displayName="Controller" extends="wheels.Global"{
375375
* @path The path to get component files from
376376
*/
377377
private function $integrateComponents(required string path) {
378-
local.basePath = arguments.path;
379-
local.folderPath = expandPath("/#replace(local.basePath, ".", "/", "all")#");
380-
381-
// Get a list of all CFC files in the folder
382-
local.fileList = directoryList(local.folderPath, false, "name", "*.cfc");
383-
for (local.fileName in local.fileList) {
384-
// Remove the file extension to get the component name
385-
local.componentName = replace(local.fileName, ".cfc", "", "all");
386-
387-
$integrateFunctions(createObject("component", "#local.basePath#.#local.componentName#"));
378+
// The directory scan + per-file createObject + getMetaData, plus the
379+
// public-method/reference resolution, are cached per path (issue #3213) —
380+
// they are identical for every controller instance. Only the reference
381+
// assignment below runs on each materialization. The mixin-override set is
382+
// resolved once per call (empty in the common no-mixins case) so the old
383+
// per-method $willBeOverriddenByMixin function call is gone from the loop.
384+
local.plan = $componentIntegrationPlan(arguments.path);
385+
local.overrideSet = $mixinOverrideSet("controller");
386+
local.iEnd = ArrayLen(local.plan);
387+
for (local.i = 1; local.i <= local.iEnd; local.i++) {
388+
$integrateFunctions(local.plan[local.i].publicMethods, local.overrideSet);
388389
}
389390
}
390391

391392
/**
392-
* Dynamically mix methods from a given component into this component
393+
* Mix a component's pre-resolved public methods (each `{name, ref}`, see
394+
* $componentIntegrationPlan) into this instance. Preserves the original
395+
* semantics: a method that does not already exist (from inheritance or an
396+
* earlier-integrated component) is added, and any method a plugin/package
397+
* mixin will override is also aliased to `super<name>`. `overrideSet` is the
398+
* precomputed mixin-override name set.
393399
*/
394-
private function $integrateFunctions(componentInstance) {
395-
// Get all methods from the given component
396-
local.methods = getMetaData(componentInstance).functions;
397-
398-
for (local.method in local.methods) {
399-
local.functionName = local.method.name;
400+
private function $integrateFunctions(required array publicMethods, required struct overrideSet) {
401+
local.iEnd = ArrayLen(arguments.publicMethods);
402+
for (local.i = 1; local.i <= local.iEnd; local.i++) {
403+
local.m = arguments.publicMethods[local.i];
404+
local.name = local.m.name;
405+
local.ref = local.m.ref;
400406

401-
// Only add public, non-inherited methods
402-
if (local.method.access eq "public") {
403-
local.methodExists = structKeyExists(variables, local.method.name) || structKeyExists(this, local.method.name);
404-
405-
if (!local.methodExists) {
406-
variables[local.functionName] = componentInstance[local.functionName];
407-
this[local.functionName] = componentInstance[local.functionName];
408-
}
409-
410-
// Only add super prefix for functions that will be overridden by plugins/mixins
411-
if ($willBeOverriddenByMixin(local.functionName)) {
412-
local.superMethodName = "super" & local.functionName;
413-
variables[local.superMethodName] = componentInstance[local.functionName];
414-
this[local.superMethodName] = componentInstance[local.functionName];
415-
}
416-
407+
if (!(StructKeyExists(variables, local.name) || StructKeyExists(this, local.name))) {
408+
variables[local.name] = local.ref;
409+
this[local.name] = local.ref;
417410
}
418-
}
419-
}
420411

421-
/**
422-
* Check if a function will be overridden by a plugin/mixin
423-
*/
424-
private boolean function $willBeOverriddenByMixin(required string functionName) {
425-
// Check if application and mixins are available
426-
if (!IsDefined("application") || !StructKeyExists(application, "wheels") || !StructKeyExists(application.wheels, "mixins")) {
427-
return false;
428-
}
429-
430-
// Check for both "controller" and "global" mixins
431-
local.componentTypes = ["controller", "global"];
432-
433-
for (local.componentType in local.componentTypes) {
434-
if (StructKeyExists(application.wheels.mixins, local.componentType) &&
435-
StructKeyExists(application.wheels.mixins[local.componentType], arguments.functionName)) {
436-
return true;
412+
if (StructKeyExists(arguments.overrideSet, local.name)) {
413+
local.superName = "super" & local.name;
414+
variables[local.superName] = local.ref;
415+
this[local.superName] = local.ref;
437416
}
438417
}
439-
440-
return false;
441418
}
442419

443420
function onDIcomplete(){

vendor/wheels/Global.cfc

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1058,6 +1058,120 @@ return local.$wheels;
10581058
return local.rv;
10591059
}
10601060

1061+
/**
1062+
* Internal. Returns a cached "integration plan" for a folder of mixin
1063+
* components (e.g. `wheels.model`, `wheels.controller`, `wheels.mapper`): an
1064+
* ordered array of `{instance, methods, fullName}` where `instance` is a
1065+
* single shared, stateless method-holder component and `methods` is its
1066+
* `getMetaData().functions` array.
1067+
*
1068+
* The directory scan, the per-file `createObject`, and the `getMetaData`
1069+
* calls are the expensive — and completely invariant — part of
1070+
* `$integrateComponents`: they produce the same result for every object of a
1071+
* given type. Before this cache they were re-paid on EVERY model, controller,
1072+
* and mapper materialization (every `new()` and every finder row goes through
1073+
* `$createInstance` -> `init()` -> `$integrateComponents`), which dominated
1074+
* test-suite and request time (issue #3213). Now they run once per path and
1075+
* the cheap per-instance work (copying function references into the target's
1076+
* `variables`/`this`) is all that remains on the hot path.
1077+
*
1078+
* The plan is cached in `application.wheels.integrationPlans`, so a reload —
1079+
* which rebuilds `application.wheels` — re-scans, the same lifetime contract
1080+
* as the schema column cache. The cached method-holder components carry no
1081+
* instance state (they are never `init()`'d) and CFML methods bind to the
1082+
* object they are invoked on, so sharing their function references across many
1083+
* target instances and across concurrent requests is safe.
1084+
*/
1085+
public array function $componentIntegrationPlan(required string path) {
1086+
// During early bootstrap (before application.wheels exists) fall back to
1087+
// an uncached build so behavior is identical to the pre-cache code path.
1088+
if (!StructKeyExists(application, "wheels")) {
1089+
return $buildComponentIntegrationPlan(arguments.path);
1090+
}
1091+
if (!StructKeyExists(application.wheels, "integrationPlans")) {
1092+
lock name="wheels.integrationPlans.#application.applicationName#" type="exclusive" timeout="10" {
1093+
if (!StructKeyExists(application.wheels, "integrationPlans")) {
1094+
application.wheels.integrationPlans = {};
1095+
}
1096+
}
1097+
}
1098+
if (!StructKeyExists(application.wheels.integrationPlans, arguments.path)) {
1099+
local.plan = $buildComponentIntegrationPlan(arguments.path);
1100+
lock name="wheels.integrationPlans.#application.applicationName#" type="exclusive" timeout="10" {
1101+
application.wheels.integrationPlans[arguments.path] = local.plan;
1102+
}
1103+
}
1104+
return application.wheels.integrationPlans[arguments.path];
1105+
}
1106+
1107+
/**
1108+
* Internal. Builds (without caching) the integration plan for a path — the
1109+
* directory scan + per-file createObject + getMetaData that
1110+
* $componentIntegrationPlan memoizes. The DirectoryList call mirrors the
1111+
* original $integrateComponents exactly so file (and therefore override)
1112+
* order is unchanged.
1113+
*/
1114+
public array function $buildComponentIntegrationPlan(required string path) {
1115+
local.folderPath = ExpandPath("/#Replace(arguments.path, ".", "/", "all")#");
1116+
local.fileList = DirectoryList(local.folderPath, false, "name", "*.cfc");
1117+
local.rv = [];
1118+
for (local.fileName in local.fileList) {
1119+
local.componentName = Replace(local.fileName, ".cfc", "", "all");
1120+
local.instance = CreateObject("component", "#arguments.path#.#local.componentName#");
1121+
local.meta = GetMetaData(local.instance);
1122+
local.fns = StructKeyExists(local.meta, "functions") ? local.meta.functions : [];
1123+
// Pre-resolve the PUBLIC method references once. On the hot path
1124+
// (every materialized object) this removes both the per-method
1125+
// `.access` filtering and the `instance[name]` scope lookup; only the
1126+
// reference assignment into the target remains (issue #3213). Function
1127+
// references are late-bound to the object they are invoked on, so the
1128+
// shared, cached reference works correctly on every target instance.
1129+
local.publicMethods = [];
1130+
local.fEnd = ArrayLen(local.fns);
1131+
for (local.f = 1; local.f <= local.fEnd; local.f++) {
1132+
if (local.fns[local.f].access == "public") {
1133+
ArrayAppend(local.publicMethods, {
1134+
name = local.fns[local.f].name,
1135+
ref = local.instance[local.fns[local.f].name]
1136+
});
1137+
}
1138+
}
1139+
ArrayAppend(local.rv, {
1140+
instance = local.instance,
1141+
methods = local.fns,
1142+
publicMethods = local.publicMethods,
1143+
fullName = StructKeyExists(local.meta, "fullName") ? local.meta.fullName : "#arguments.path#.#local.componentName#"
1144+
});
1145+
}
1146+
return local.rv;
1147+
}
1148+
1149+
/**
1150+
* Internal. Returns a struct whose KEYS are the function names that a
1151+
* registered plugin/package mixin will override for the given component type
1152+
* (plus the always-checked "global" type). Empty — the common case, no mixins
1153+
* registered — when there are none. Computed from the app-scoped, reload-stable
1154+
* application.wheels.mixins so the per-method $willBeOverriddenByMixin function
1155+
* call can be replaced by an O(1) struct-membership test on the hot path (#3213).
1156+
*/
1157+
public struct function $mixinOverrideSet(required string primaryType) {
1158+
local.rv = {};
1159+
if (
1160+
!StructKeyExists(application, "wheels")
1161+
|| !StructKeyExists(application.wheels, "mixins")
1162+
|| StructIsEmpty(application.wheels.mixins)
1163+
) {
1164+
return local.rv;
1165+
}
1166+
local.types = [arguments.primaryType, "global"];
1167+
for (local.t in local.types) {
1168+
if (StructKeyExists(application.wheels.mixins, local.t) && IsStruct(application.wheels.mixins[local.t])) {
1169+
StructAppend(local.rv, application.wheels.mixins[local.t], false);
1170+
}
1171+
}
1172+
return local.rv;
1173+
}
1174+
10611175
/**
10621176
* Internal function.
10631177
*/

vendor/wheels/Mapper.cfc

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -379,38 +379,50 @@ component output="false" {
379379
* @path The path to get component files from
380380
*/
381381
private function $integrateComponents(required string path) {
382-
local.basePath = arguments.path;
383-
local.folderPath = expandPath("/#replace(local.basePath, ".", "/", "all")#");
384-
385-
// Get a list of all CFC files in the folder
386-
local.fileList = directoryList(local.folderPath, false, "name", "*.cfc");
387-
for (local.fileName in local.fileList) {
388-
// Remove the file extension to get the component name
389-
local.componentName = replace(local.fileName, ".cfc", "", "all");
390-
391-
$integrateFunctions(createObject("component", "#local.basePath#.#local.componentName#"));
392-
}
382+
// The directory scan + per-file createObject + getMetaData, plus the
383+
// public-method/reference resolution, are cached per path (issue #3213).
384+
// The `get`/`controller` exclude-list only applies to NON-wheels.mapper
385+
// sources, so for the wheels.mapper.* components scanned here every public
386+
// method is integrated — exactly what the precomputed publicMethods hold.
387+
local.plan = $componentIntegrationPlan(arguments.path);
388+
local.iEnd = ArrayLen(local.plan);
389+
for (local.i = 1; local.i <= local.iEnd; local.i++) {
390+
$integrateFunctions(local.plan[local.i].instance, local.plan[local.i].publicMethods);
391+
}
393392
}
394393

395394
/**
396-
* Dynamically mix methods from a given component into this component.
397-
* Only public, non-inherited methods are added.
395+
* Mix a component's methods into this component. The cached path passes the
396+
* pre-resolved public methods (each `{name, ref}`, see
397+
* $componentIntegrationPlan) and assigns them directly. The fallback path —
398+
* used by init() integrating wheels.Global with no cached list — keeps the
399+
* original metadata scan plus the `get`/`controller` exclude-list (#3213).
398400
*
399401
* @param componentInstance The component instance to integrate methods from.
400402
*/
401-
private function $integrateFunctions(required any componentInstance) {
402-
// Get metadata for the component
403-
local.methods = getMetaData(componentInstance).functions;
404-
local.componentName = getMetaData(componentInstance).FULLNAME;
403+
private function $integrateFunctions(required any componentInstance, array publicMethods = []) {
404+
// Cached path: pre-resolved public method references.
405+
if (ArrayLen(arguments.publicMethods)) {
406+
local.iEnd = ArrayLen(arguments.publicMethods);
407+
for (local.i = 1; local.i <= local.iEnd; local.i++) {
408+
local.m = arguments.publicMethods[local.i];
409+
variables[local.m.name] = local.m.ref;
410+
this[local.m.name] = local.m.ref;
411+
}
412+
return;
413+
}
405414

406-
// Iterate over the functions in the component
415+
// Fallback (e.g. init() integrating wheels.Global): scan metadata and
416+
// apply the exclude-list against the source's full name.
417+
local.meta = getMetaData(arguments.componentInstance);
418+
local.methods = StructKeyExists(local.meta, "functions") ? local.meta.functions : [];
419+
local.componentName = StructKeyExists(local.meta, "fullName") ? local.meta.fullName : "";
407420
for (local.method in local.methods) {
408421
local.functionName = local.method.name;
409422
local.excludeList = "get,controller";
410423

411-
// Add only public, non-inherited methods excluding specific ones
424+
// Add only public methods, excluding specific ones unless the source is a mapper component.
412425
if (local.method.access == "public" && (!listFindNoCase(local.excludeList, local.functionName) || findNoCase("wheels.mapper", local.componentName))) {
413-
// Assign methods to `variables` and `this`
414426
variables[local.functionName] = componentInstance[local.functionName];
415427
this[local.functionName] = componentInstance[local.functionName];
416428
}

0 commit comments

Comments
 (0)