Skip to content

Conversation

smnandre
Copy link
Member

Currently if we have 50 times the same component in a page, 50 times we use reflection to analyse component class properties and methods.

This PR centralize this task in a dedicated (internal) service and add a cachewarmer to pre-compute metadata during app build.

Significant performance gains here too
(i won't do charts for every PR but be sure i'm gonna make some before/after once i'm "done" with all this.... in Vienna 👼 )

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Sep 26, 2024
Copy link
Contributor

@WebMamba WebMamba left a comment

Choose a reason for hiding this comment

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

Looks pretty cool! Thanks @smnandre

@onEXHovia
Copy link
Contributor

As an alternative, or even as a follow-up, I think it makes sense to replace/improve class ComponentMetadata. We could get all possible meta information PostMount/PreMount/ExposeInTemplate hooks etc, not just about properties.

Structure may look like this:
ComponentMetadata - store metadata
ComponentMetadataFactory - provider metadata with methods getMetadataFor/hasMetadataFor. Factory methods return ComponentMetadata instance.
ComponentCacheWarmer - cache warmer call factory ComponentMetadataFactory

Copy link
Contributor

@WebMamba WebMamba left a comment

Choose a reason for hiding this comment

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

Are we agree, this is happening only in prod ? I don't see anything prod related

@smnandre
Copy link
Member Author

Hey @onEXHovia ! You read my mind

This is indeed my next move, and i started to extract the props metadata from template too :)

@smnandre
Copy link
Member Author

Are we agree, this is happening only in prod ?

@WebMamba nothing "prod-related"

For all the environments, during a request, there is no need to compute metadata twice, so the values are "kept".

Also, if you use cache:warm all the components metadata are cached in the ssytem cache.

What's maybe missing here is something to invalidate the cache locally if you change a file ?

Do you have encountered any trouble ?

@Kocal
Copy link
Member

Kocal commented Sep 29, 2024

image

A small rebase before merging? :) 🙏🏻

@smnandre smnandre force-pushed the dev/component-properties branch 2 times, most recently from 1c2fdae to aca73a5 Compare October 5, 2024 11:15
@carsonbot carsonbot added Status: Needs Review Needs to be reviewed and removed Status: Reviewed Has been reviewed by a maintainer labels Oct 7, 2024
@smnandre smnandre force-pushed the dev/component-properties branch from aca73a5 to 0880687 Compare October 22, 2024 23:07
@smnandre smnandre changed the title [TwigComponent] Centralize, reuse and warm-up component properties metadata [TwigComponent] Centralize, reuse and warm-up component properties metadata [WIP] Oct 22, 2024
@smnandre smnandre force-pushed the dev/component-properties branch from 0880687 to 17ebd17 Compare November 1, 2024 06:02
@smnandre smnandre changed the title [TwigComponent] Centralize, reuse and warm-up component properties metadata [WIP] [TwigComponent] Cache component properties metadata Nov 1, 2024
@smnandre smnandre force-pushed the dev/component-properties branch from 17ebd17 to 049be46 Compare November 1, 2024 06:07
@smnandre smnandre force-pushed the dev/component-properties branch from 049be46 to 1cc7362 Compare November 1, 2024 06:57
@smnandre smnandre merged commit ad8ac6a into symfony:2.x Nov 1, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants