Improvement: Remove Cached JSON File Reading for Faster Performance

[alissn]

Hi,

In this pull request, I propose removing the cached JSON file reading and instead reading directly from the file. This approach is significantly faster (~12x on my OS).

Before merging, please test this on your own system.

How to Test This

1. Create a new fresh Laravel project and generate 150 modules. Use the following console command:

   use Illuminate\Support\Facades\Artisan;
   
   Artisan::command('create', function () {
       $module_names = collect(range(0, 150))
           ->map(function ($item) {
               return 'module' . $item;
           })
           ->toArray();
   
       \Illuminate\Support\Facades\Artisan::call('module:make', [
           'name' => $module_names,
       ]);
   });

   Run this command in the terminal:

   php artisan create

2. Publish the module config file and set cache.enabled to true or set the environment variable MODULES_CACHE_ENABLED=true.

3. Create a benchmark route with this code:

   Route::get('/benchmark', function () {
       $base_path = base_path();
   
       \Illuminate\Support\Benchmark::dd([
           'directly from file' => function () use ($base_path) {
               $num = rand(0, 150);
               $path = "$base_path/Modules/Module{$num}/module.json";
               $json = \Illuminate\Support\Facades\File::json($path);
               $name = $json['name'];
           },
           'read from cache' => function () use ($base_path) {
               $num = rand(0, 150);
               $path = "$base_path/Modules/Module{$num}/module.json";
               $json = app('cache')
                   ->store(config('modules.cache.driver'))
                   ->remember($path, 100000, function () use ($path) {
                       return \Illuminate\Support\Facades\File::json($path);
                   });
               $name = $json['name'];
           },
       ], 1000);
   });

Benchmark Results

Here are my results on redis cache is:

on file driver and, first run :

array:2 [▼ 
  "directly from file" => "0.014ms"
  "read from cache" => "0.109ms"
]

after 3 4 run, result improve:

array:2 [▼ 
  "directly from file" => "0.014ms"
  "read from cache" => "0.079ms"
]

[solomon-ochepa]

Regarding the issue #1745, completely eradicating the caching system is not advisable. The ability to disable or enable it when needed is sufficient. By default, it is disabled, which effectively makes it inactive, but it allows those who find it useful to activate it when necessary.

I recommend testing other scenarios, such as those mentioned in #1745 (comment), #1745 (comment), #1745 (comment), #1745 (comment), for potential improvements.

When proposing a change or feature like this, consider its impact on others. The best practice is to provide a configuration option to enable or disable the feature as needed, and this element meets that requirement.

Thank you for your contribution.

[alissn]

@solomon-ochepa

Did you review the changes and test this pull request?

This pull request only deletes the cache of the JSON file of the modules (NOT ALL CACHE) and does not address the mentioned issue.

[solomon-ochepa]

@solomon-ochepa

Did you review the changes and test this pull request?

Yes, sir. I did.

This pull request only deletes the cache of the JSON file of the modules (NOT ALL CACHE) and does not address the mentioned issue.

You asked that the getAttributes() method return $this->decodeContents() without checking the condition - if (config('modules.cache.enabled') === false).

"The ability to disable or enable it when needed is sufficient. By default, it is disabled, which effectively makes it inactive, but it allows those who find it useful to activate it when necessary."

[alissn]

@solomon-ochepa

Please follow the "How to Test This" section in the description and share your results.

The default cache driver is file, which creates a cache file from the JSON file and reads from the cached file using the cache driver. Do you think this is correct and should be kept?

[solomon-ochepa]

@solomon-ochepa

Please follow the "How to Test This" section in the description and share your results.

The default cache driver is file, which creates a cache file from the JSON file and reads from the cached file using the cache driver. Do you think this is correct and should be kept?

Does this happen even when "modules.cache.enabled" == false?

[solomon-ochepa]

@solomon-ochepa

Please follow the "How to Test This" section in the description and share your results.

The default cache driver is file, which creates a cache file from the JSON file and reads from the cached file using the cache driver. Do you think this is correct and should be kept?

Your test ignored the condition if(config("modules.cache.enabled" == false){...} and read from cache directly - that is wrong testing!

"... By default, it is disabled, which effectively makes it inactive, but it allows those who find it useful to activate it when necessary."

[dcblogdev]

@alissn Reading from file is constantly faster, I ran 150 modules and ran the benchmarks. I don't see any obvious downside to this PR, this alone won't solve the speed issue when there's 150+ modules.

Disabled modules won't be loaded, this only effects active modules as you already mentioned.

array:2 [▼
  "directly from file" => "0.013ms"
  "read from cache" => "0.166ms"
]

array:2 [▼
  "directly from file" => "0.013ms"
  "read from cache" => "0.078ms"
]

array:2 [▼
  "directly from file" => "0.013ms"
  "read from cache" => "0.077ms"
]

array:2 [▼
  "directly from file" => "0.014ms"
  "read from cache" => "0.063ms"
]

array:2 [▼
  "directly from file" => "0.013ms"
  "read from cache" => "0.078ms"
]

array:2 [▼
  "directly from file" => "0.013ms"
  "read from cache" => "0.080ms"
]

[alissn]

Hi,

This pull request aims to improve the speed of module loading by reading files directly from the modules. It's part of a series of pull requests focused on enhancing performance. In subsequent pull requests, I'll address other sections to further speed up loading.

Thanks.