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.

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

[FrameworkBundle] Register the messenger data collector only when the profiler is enabled #28418

Merged
merged 1 commit into from Sep 10, 2018

Conversation

Projects
None yet
8 participants
@pierredup
Copy link
Contributor

commented Sep 10, 2018

Q A
Branch? 4.1
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #28350
License MIT
Doc PR N/A

The data collector for the messenger is currently unconditionally registered, which causes increased memory usage even in production. Instead, it should only be registered along with the rest of the data collectors only when the profiler is enabled

@pierredup pierredup changed the base branch from master to 4.1 Sep 10, 2018

@sroze

sroze approved these changes Sep 10, 2018

@sroze

This comment has been minimized.

Copy link
Member

commented Sep 10, 2018

Thank you @pierredup.

@sroze sroze merged commit bd3a66b into symfony:4.1 Sep 10, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

sroze added a commit that referenced this pull request Sep 10, 2018

bug #28418 [FrameworkBundle] Register the messenger data collector on…
…ly when the profiler is enabled (pierredup)

This PR was merged into the 4.1 branch.

Discussion
----------

[FrameworkBundle] Register the messenger data collector only when the profiler is enabled

| Q             | A
| ------------- | ---
| Branch?       | 4.1
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #28350
| License       | MIT
| Doc PR        | N/A

The data collector for the messenger is currently unconditionally registered, which causes increased memory usage even in production. Instead, it should only be registered along with the rest of the data collectors only when the profiler is enabled

Commits
-------

bd3a66b Register the messenger data collector only when the profiler is enabled

@sroze sroze added the Messenger label Sep 10, 2018

@pierredup pierredup deleted the pierredup:messenger branch Sep 10, 2018

chalasr added a commit that referenced this pull request Sep 16, 2018

minor #28474 [FrameworkBundle] Don't register MessengerDataCollector …
…if messenger is not enabled (chalasr)

This PR was merged into the 4.1 branch.

Discussion
----------

[FrameworkBundle] Don't register MessengerDataCollector if messenger is not enabled

| Q             | A
| ------------- | ---
| Branch?       | 4.1
| Bug fix?      | no (not yet released)
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Will fix `deps=high` SecurityBundle's build once merged up to master which is broken since #28418

Commits
-------

e64ceb5 [FrameworkBundle] Don't register MessengerDataCollector if messenger is not enabled
@konradja100

This comment has been minimized.

Copy link

commented Sep 24, 2018

Hi guys, how to test this patch in existing project? as i asume this bugfix will be released in 4.1.5 right?

@sroze

This comment has been minimized.

Copy link
Member

commented Sep 24, 2018

Yes, it will. In the meantime, you could use the version 4.1@dev in Composer, it should get you the 4.1 branch that contains this change.

@fabpot fabpot referenced this pull request Sep 30, 2018

Merged

Release v4.1.5 #28642

@zeves095

This comment has been minimized.

Copy link

commented Nov 1, 2018

v4.1.6 ) do'nt release memory in prod: http://prntscr.com/ld5yu3

@chalasr

This comment has been minimized.

Copy link
Member

commented Nov 1, 2018

@zeves095 Can you open a new issue with enough details to reproduce?

@zeves095

This comment has been minimized.

Copy link

commented Nov 2, 2018

@zeves095 Can you open a new issue with enough details to reproduce?

#29067

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.