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 enhancement snippet (pre PR discussion) #66

Open
cjke opened this issue Mar 14, 2018 · 0 comments
Open

Performance enhancement snippet (pre PR discussion) #66

cjke opened this issue Mar 14, 2018 · 0 comments

Comments

@cjke
Copy link
Contributor

cjke commented Mar 14, 2018

The following is a working (but with possible regressions) snippet to reduce the query load of fetching many acf fields.

Summary

The overarching principal is:

  1. pass in an array of key/value pairs (the field is key, as the type is not likely to be unique)
  2. a new method ("fields" or "bulk" or whatever) makes a generic select * from wp_postmetawherepost_id in ('106') via $this->post->meta
  3. we filter down the meta values to just the keys we passed in
  4. feed those meta models back into the field factory.

This, from elementary testing, changes the query to 1 + N (1 for post + N per acf field) to 2 (1 for post + 1 for all meta).

Before:

image

After:

image

Code changes

    // Sample usage
    Staff::find(1);
    $fields = $staff->acf->fields([
        'staff_short_description' => 'wysiwyg',
        'staff_description' => 'wysiwyg',
        'staff_role' => 'text',
    ]);

    dd($fields); // (see screenshot)

The following ACF fields would need to be modified:

    // AdvancedCustomFields.php
    // Add new bulk/fields/etc method
    public function fields($fields)
    {
        // whereIn limits the fields down to just the keys we are looking for
        // feed the same stuff into the Factory
        return $this->post
            ->meta
            ->whereIn('meta_key', array_keys($fields))
            ->mapWithKeys(function($field) use ($fields)
            {
                $value = FieldFactory::make($field, $this->post, snake_case($fields[$field->meta_key]))->get();
                return [$field->meta_key => $value];
            });
    }

And BasicField (at minimum):

    // BasicField
    // This bit is the tricky bit - we need to remove the query from fetchValue 
    // possibly into a seperate method (maybe performQuery)
    public function fetchValue($field)
    {
    // $postMeta = $this->postMeta->where(
    //    $this->getKeyName(), $this->post->getKey()
    // )->where('meta_key', $field)->first();

    if (isset($postMeta->meta_value) and ! is_null($postMeta->meta_value)) {
        $value = $postMeta->meta_value;
        ...
        // remaining code is the same

Concerns

My major concern is I am missing something major - as to why the per field query is needed (the removed lines in BasicField).

Obviously regression is reasonably big too.

Thoughts before kicking off a PR? I'm extremely short on time (who isn't?) at the moment - so need to be careful how I allocate resources.

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

1 participant