wp-graphql / wp-graphql-acf

WPGraphQL for Advanced Custom Fields
https://wpgraphql.com/acf
627 stars 123 forks source link

High CPU usage for get_location_rules() inside `add_acf_fields_to_graphql_types` #362

Closed ovidiul closed 1 year ago

ovidiul commented 1 year ago

While looking at the performance of one of our websites GraphQL enpoint, we've found out that this line in particular in causing an increase in CPU usage making our site unstable when a high number of requests to the graphql endpoint is sent.

Since that value is already initialised for that class on line 71, can that original code

$location_rules               = $this->get_location_rules();
if ( isset( $location_rules[ $field_group_name ] ) ) {
    $field_group['graphql_types'] = $location_rules[ $field_group_name ];
}

be replace with:

if ( isset( $this->location_rules[ $field_group_name ] ) ) {
    $field_group['graphql_types'] = $this->location_rules[ $field_group_name ];
}

or, at the very least, taken out of that foreach loop.

The performance improvements on our site are drastically visible, from an initial 25+ seconds response time down to <2s after the change.

Thanks for considering this, happy to draft a PR if needed.

jasonbahl commented 1 year ago

@ovidiul 👋🏻 hey, we'll be archiving this repo in the not-too-distant future in favor of the re-written plugin here: https://github.com/wp-graphql/wpgraphql-acf which will be moving from beta to stable shortly.

I'd be curious to see if you experience the same issue on the new version of the plugin or if you'd consider this resolved in the new plugin (the codebase is significantly different).

jasonbahl commented 1 year ago

I'm going to close this as we won't be addressing this in the current repo as our attention is focused on the new version.

If this is determined to be an issue in the new version of the plugin, please open an issue in that repo and we'll look into it further 🙏🏻