Skip to content

refactor-queryCallback-with-inbuilt-query-functions #1223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

chapmandu
Copy link
Contributor

@chapmandu chapmandu commented Nov 14, 2022

  • Speed this up by eliminating list functions
  • Use ACF2016+ inbuilt query functions

Performance test results:

(Before) With callback (ListFindNoCase)
afterfind-callback-ListFindNoCase 15145ms

(After) With callback (QueryKeyExists)
afterfind-callback-QueryKeyExists 1474ms

Without callback (afterfind callbacks are evil due to query row to struct creation on each row)
without-callback 232ms

Test code

// test.cfm
cftimer(label = "afterfind", type = "debug") {
	loop from=1 to=20 index="i" {
		model("User").findAll(maxrows = 100, reload = true);
	}
}

// models/User.cfc
afterFind("do");
...
function foo() {
	arguments.foo = true;
	arguments.bar = true;
	arguments.baz = true;
	arguments.qux = true;
	return arguments;
}

@bpamiri
Copy link
Collaborator

bpamiri commented Mar 15, 2023

@chapmandu are you still working on this PR?

@chapmandu
Copy link
Contributor Author

I'll have another look today and see where it's at.

@chapmandu chapmandu marked this pull request as ready for review March 16, 2023 09:54
@chapmandu chapmandu marked this pull request as draft March 16, 2023 09:54
@chapmandu
Copy link
Contributor Author

I'm going to do some performance comparisons to see what gains are to be had.

@chapmandu chapmandu marked this pull request as ready for review March 16, 2023 22:37
@bpamiri bpamiri merged commit d024e39 into cfwheels:develop Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants