Skip to content
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

Performance regression due to additional count call #242

Closed
bwaidelich opened this issue Dec 19, 2016 · 5 comments
Closed

Performance regression due to additional count call #242

bwaidelich opened this issue Dec 19, 2016 · 5 comments

Comments

@bwaidelich
Copy link

Calling count() on an object can be very "expensive", especially if it's a QueryResult-object with a large set of records as a SELECT COUNT(*) can be very slow with a lot of records (pagination, i.e. a limit/offset won't make a difference).

In my case it's more than an order of a magnitude difference to render 30 records at once (0,042s vs 0,76s)

The bottleneck comes with the newly introduced AbstractViewHelper::isValidType():
It does some (possibly dangerous and expensive) operations on the given arguments, including count and getFirstElementOfNonEmpty() on collection values.

Previously we only checked whether the type matched the argument definition

BTW: There's another possible side-effect in the argument validation: getFirstElementOfNonEmpty() actually resets any collection argument

@bwaidelich
Copy link
Author

(reference: neos/flow-development-collection#759)

@bwaidelich
Copy link
Author

FYI: A quick fix is to replace

} elseif (count($value) > 0 && substr($type, -2) === '[]') {

by

} elseif (substr($type, -2) === '[]' && count($value) > 0) {

in AbstractViewHelper::isValidType()

Which makes sense anyways IMO - but obvously that's just a fix for some of the cases

@NamelessCoder
Copy link
Member

I'm thinking we may be able to drop the count() entirely in the case above. I'm looking into that right now.

@NamelessCoder
Copy link
Member

#245 could be a way to deal with the count() in argument validation. It does however slightly change the verdict for non-empty arrays/iterators where the first value is null, for whichever reason. Such cases are now considered valid (taken as reasonable indication that the subject was empty, thus valid).

NamelessCoder added a commit to NamelessCoder/Fluid that referenced this issue Dec 19, 2016
This prevents calling count() on argument to check validity
of first item. Cases where first value is "null" now means
that the value itself is valid (e.g. container is empty).

Close: TYPO3#242
@bwaidelich
Copy link
Author

Great, I like!

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

No branches or pull requests

2 participants