orlyapps / nova-belongsto-depend

Larave Nova BelongsTo Field with Dependcy
MIT License
182 stars 65 forks source link

Eager Loading not respected ! #66

Open issour opened 4 years ago

issour commented 4 years ago

Many Thx

orlyapps commented 4 years ago

Hello, thanks for the issue. Can you explain it a bit further?

abdulfattah-tayih-mawdoo3 commented 4 years ago

I am facing the same issue, the field always lazy loads the relation data which makes the page load very slow.

To clarify, even if I add all the required relations in the $with parameter of the nova resource, the NovaBelongsToDepend field will always load all relations for each record individually, which creates the infamous n + 1 problem

alexjose commented 3 years ago

image

image

eugenefvdm commented 3 years ago

I have to unfortunately agree with some of the users here that there seems to be a N+1 problem. We have a client related model with 30 000 records, and nova-belongsto-depend just doesn't work for that page. On a slow computer it can take up to a minute, and on a really fast server always 20 seconds. We tried using a caching plugin https://github.com/GeneaLabs/laravel-model-caching but that didn't seem to make a difference. I think the problem isn't so much to do with this module, but also with the way Nova works. It really seems somewhat inefficient at times. But anyway, I'm documenting it here.

To illustrate this as the user above, here is my setup But note in the screenshot that each select id,name from client fetching 30K record. To see where optimization can be applied, I tried:

What's a bit interesting is that the package doesn't seem to use a cached query.

So there it is, a slow page that I can't give to my client.

download

cwilby commented 3 years ago

Having this problem, only gets worse with a deeply nested hierarchy. Going to look into this some today.

cwilby commented 3 years ago

Ok, managed to get ~1500% performance increase with the following fixes:

My structure is Countries -> States -> State Divisions -> Regions -> Cities -> Facilities -> ExerciseGroup. There are a lot of groups, so loading that index view was taking a long time in production with all the n+1 going on.

1. Patch NovaBelongsToDepend::resolve

When Nova goes to resolve a field value (e.g. Facility for ExerciseGroup), this package currently fetches the related value (Facility) from the database without checking if it was previously eager loaded.

This patch yanks the logic from parent::resolve to check if the attribute being resolved is a relation that was previously eager loaded. It enables the next fix to work correctly.

https://github.com/cwilby/nova-belongsto-depend/commit/6944f62c628911336f4edbf9f0f2d46c75be123c

PR here: https://github.com/orlyapps/nova-belongsto-depend/pull/86

Update, PR merged

2. Set $with on each resource in your hierarchy.

After applying the patch, each resource should specify $with values, so that when fields are resolved for that resource, the models are in memory.

In my case: App\Nova\ExerciseGroup specifies public static $with = ['facility', 'city', 'region', 'division', 'state', 'country']; App\Nova\Facility specifies public static $with = ['city', 'region', 'division', 'state', 'country']; ..and so on.

3. Cache root node of the hierarchy

In my case, my root is Country. Because these options are resolved when the resource fields are fetched, there will be a query for each row to be returned to the index view, and recursively for related resources.

I didn't see another way around this other than caching.

NovaBelongsToDepend::make('Country')
+    ->options(Cache::remember('countries', 60, fn () => Country::all()))
-    ->options(Country::all())

4. (Optional but related) Manually get relation values to fix sorting

Sorting is currently applied to the foreign key rather than a field. To get around this, add an indexQuery method that left joins the related records and select fields as {{relation}}_name.

public static function indexQuery(NovaRequest $request, $query)
{
    return $query
        ->leftJoin('countries', 'countries.id', '=', 'exercise_groups.country_id')
        ->leftJoin('states', 'states.id', '=', 'exercise_groups.state_id')
        ->leftJoin('state_divisions', 'state_divisions.id', '=', 'exercise_groups.state_division_id')
        ->leftJoin('regions', 'regions.id', '=', 'exercise_groups.region_id')
        ->leftJoin('cities', 'cities.id', '=', 'exercise_groups.city_id')
        ->leftJoin('facilities', 'facilities.id', '=', 'exercise_groups.facility_id')
        ->select(
            'exercise_groups.*',
            'countries.name as country_name',
            'states.name as state_name',
            'state_divisions.name as state_division_name',
            'regions.name as region_name',
            'cities.name as city_name',
            'facilities.name as facility_name'
        );
}

Then, for each NovaBelongsToDepend field, add ->withMeta(['sortableUriKey' => 'relation_name']);

NovaBelongsToDepend::make('City')
    ->dependsOn('Region')
    ->optionsResolve(fn ($region) => $region->cities)
    ->showCreateRelationButton()
    ->nullable()
    ->sortable()
+    ->withMeta(['sortableUriKey' => 'city_name'])

Results

Before (307 queries, 290 duplicate, 303 models, 16MB RAM, 2170ms)

image

After (15 queries, 0 duplicate, 96 models, 15MB RAM, 190ms)

image