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.

Sign up for GitHub

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

Jump to bottom

perf(laravel): Caching tables and indexes to avoid multiple queries #6744

Closed
wants to merge 6 commits into from
Closed

perf(laravel): Caching tables and indexes to avoid multiple queries #6744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b766203
perf(laravel): Caching tables and indexes to avoid multiple queries
amermchaudhary Oct 23, 2024
cc206e8
perf(laravel) Updates WorkbenchServiceProvider to use null for the de…
amermchaudhary Oct 23, 2024
898ab51
perf(laravel): making phpstan happy
amermchaudhary Oct 23, 2024
b6068fc
perf(laravel) fixing the return type for phpstan
amermchaudhary Oct 23, 2024
618785a
perf(laravel): changing the return type for phpstan
amermchaudhary Oct 23, 2024
a23ebcc
perf(laravel): making phpstan happier
amermchaudhary Oct 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/Laravel/Eloquent/Metadata/ModelMetadata.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\Relation;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Gate;
use Illuminate\Support\Str;

Expand Down Expand Up @@ -65,8 +66,14 @@ public function getAttributes(Model $model): Collection
$connection = $model->getConnection();
$schema = $connection->getSchemaBuilder();
$table = $model->getTable();
$columns = $schema->getColumns($table);
$indexes = $schema->getIndexes($table);
/** @var array<string, mixed> $columns */
$columns = Cache::flexible('api-platform.tables.'.$table, [5, 10], function () use ($schema, $table) {
return $schema->getColumns($table);
});
/** @var array<int, mixed> $indexes */
$indexes = Cache::flexible('api-platform.indexes.'.$table, [5, 10], function () use ($schema, $table) {
return $schema->getIndexes($table);
});
Copy link
Member

@soyuka soyuka Oct 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting change, note also #6735. We already have a cache layer so that this isn't called more then once per metadata computation in production mode.

This cache is stored in memory ? File ? How do you make sure it's not stale ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@soyuka the storage engine of the cache is configurable from laravel's cache config in the file config/cache.php. You can use any cache driver (file, database, redis, memcached, etc) and the second argument to the function [5, 10] basically tells laravel that the cache is only good for 5 seconds and between 5 to 10 seconds period refresh the cached value automatically so it'll only be at max 5 to 10 seconds old.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@soyuka I logged all the queries and saw multiple calls to the information_schema table for the same table's columns and data and thats why I added this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@soyuka in terms of making sure it is not stale:
https://laravel.com/docs/11.x/cache#swr

But as you pointed out, in production this is not called because it cached when calling the optimize function (or after the 1st request to the endpoint), so this would apply only to development machines, im not quite sure if or why it would be needed there.

@amermchaudhary Are you running into this issue on a production setup or on your development system?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes @amermchaudhary it's running multiple time on a development environment, but if you set debug to false metadata gets cached. I'm not a huge fan of multi-layer caching as it's hard to debug when it fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh I was checking on my development system. I didn't know that it was being cached on a production system. In that case we should close this PR. It's not necessary!!


return collect($columns)
->map(fn ($column) => [
Expand Down
1 change: 1 addition & 0 deletions src/Laravel/workbench/app/Providers/WorkbenchServiceProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public function register(): void
{
$config = $this->app['config'];
$config->set('api-platform.resources', [app_path('Models'), app_path('ApiResource')]);
$config->set('cache.default', 'null');
}

/**
Expand Down
Loading