Skip to content

Conversation

@smnandre
Copy link
Member

@smnandre smnandre commented Nov 6, 2024

ComponentFactory Quick Optimization

Minor change with.. mid impact
(on large number of renders)

@smnandre
Copy link
Member Author

smnandre commented Nov 6, 2024

false positive for fabbot

@norkunas
Copy link
Contributor

norkunas commented Nov 6, 2024

When running let's say frankenphp, this will always stay in memory?

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Nov 6, 2024
@Kocal
Copy link
Member

Kocal commented Nov 6, 2024

Thanks for this optimization!

However I'm thinking, not now but maybe for a next PR, but do you think things can be more improved if this "find mounts methods" process was done during a cache warmer and persisted in cache?
I don't speak about storing ReflectionMethod in cache (I don't think it is possible), but storing that if $component::class has mount method or not:

    private function mount(object $component, array &$data): void
    {
        if ($component instanceof AnonymousComponent) {
            $component->mount($data);

            return;
        }
 
+       if (!self::$hasMountMethod[$component::class]) {
+           return;
+       }

        if (!\array_key_exists($component::class, self::$mountMethods)) {
            try {
                $mountMethod = self::$mountMethods[$component::class] = (new \ReflectionClass($component))->getMethod('mount');
            } catch (\ReflectionException) {
                self::$mountMethods[$component::class] = false;

                return;
            }
        }

WDYT?

@smnandre
Copy link
Member Author

smnandre commented Nov 6, 2024

However I'm thinking, not now but maybe for a next PR [...] WDYT?

Absolutely!

I'm moving step by step, a bit in the blind. The biggest change is yet to come (this week hopefully). This is why i'm only doing work on private/internal methods for now.

Once we have the results i'm targetting, a clean will be very welcome (required?).. We will then see how to name these classes / methods, and how we can rearrange the whole thing

👍

@smnandre
Copy link
Member Author

smnandre commented Nov 6, 2024

When running let's say frankenphp, this will always stay in memory?

Would it be a problem ? Genuine question here :)

@norkunas
Copy link
Contributor

norkunas commented Nov 6, 2024

Depends on how much memory it will use. ResetInterface could be used to cleanup after request

@smnandre
Copy link
Member Author

smnandre commented Nov 6, 2024

Updated!

@smnandre smnandre force-pushed the twig/opti-mount-methods branch from 2298e19 to 6486933 Compare November 10, 2024 01:02
@smnandre smnandre merged commit 7dff0df into symfony:2.x Nov 10, 2024
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Performance Status: Reviewed Has been reviewed by a maintainer TwigComponent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants