[TwigComponent][Perf] Remove component dumping from profiler to reduce memory usage #3066
Conversation
carsonbot
added
Status: Reviewed
|
So i checked on the project I work on and .... I'm not a big fan of this idea (but not against the idea of not storing the components ... but) Multiple data are only visible here in the profiler... and basic scalar data is not something that would take that much memory (at all). Could you maybe create a repository / share some code (or context) to understand what numver of components (or what amount of inner data) becomes a problem in dev for you ? I'd rather we change profiling and remove an important feature of it only if we have a concrete example to base this choice on.... |
|
Since having components dumps in the profiler is useful but could become a mess on memory usage when having too much components, what about introducing a new option This option could be |
100%, something I suggested here but forgot to move back here (my bad) |
|
Status: Needs work |
carsonbot
added
Status: Needs Work
alinceDev
force-pushed
the
twig-component-profiler
branch
from
600e66f to
6da25e6
Compare
carsonbot
added
Status: Needs Review
alinceDev
force-pushed
the
twig-component-profiler
branch
2 times, most recently
from
ac7093d to
21d69fc
Compare
alinceDev
force-pushed
the
twig-component-profiler
branch
from
21d69fc to
36c66b4
Compare
| ->args([ | ||
| service('ux.twig_component.component_logger_listener'), | ||
| service('twig'), | ||
| param('ux.twig_component.profiler_dump_components'), |
There was a problem hiding this comment.
I would use an abstract arg here, and define the parameter on the definition after the load(debug.php) in the extension class... not sure we should create a parameter here
| $defaults = []; | ||
| } | ||
| $container->setParameter('ux.twig_component.component_defaults', $defaults); | ||
| $container->setParameter('ux.twig_component.profiler_dump_components', $config['profiler_dump_components']); |
| ->booleanNode('profiler_dump_components') | ||
| ->info('Enables the dump components for Twig Component (in debug mode)') | ||
| ->defaultValue('%kernel.debug%') | ||
| ->end() |
There was a problem hiding this comment.
defaultTrue() seems more logical here .. (its not just in debug mode but... when twig component profiler is enabled)
Without the parenthesis, probably something like
"Dump components in the Twig Component profiler panel"
wdyt ?
|
Also could you add a tests or two, you can find similar ones in tests/Unit/DependencyInjection/TwigComponentExtensionTest.php and tests/Unit/DataCollector/TwigComponentDataCollectorTest.php |
5 participants
This PR removes the component object dumping from the Symfony UX Twig Component profiler to address memory consumption issues.
IMHO, the component dump is not necessary in the profiler as it can be quite verbose and memory-intensive. When detailed component inspection is needed, it's preferable to dump the component manually in the specific context where debugging is required.