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

Add meaning of yellow icon for number of queries #4842

Closed
wants to merge 2 commits into from
Closed

Add meaning of yellow icon for number of queries #4842

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 15, 2015

Some people (me included) have been confused about why the number of queries is sometimes displayed in a yellow color. Added explanation for this.

Number is from https://stackoverflow.com/questions/17859176/what-does-the-yellow-color-warning-icon-mean-in-symfony2-web-profiler

If you make more than 50 queries on a page, the icon will turn yellow to
let you know that you are making a lot of requests to the database. This
is only a warning and does not have any effect on the execution of further
queries.
Copy link
Member

Choose a reason for hiding this comment

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

It is only a warning, but it means there is potentially an issue in your page (N+1 requests because of proxy initialization in a loop for instance). The warning is here for a reason. Such number of queries is often an issue

Copy link
Author

Choose a reason for hiding this comment

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

Yes. Are you saying here we should tell the user more aggressively that they should keep the number of queries below 50?

Copy link
Member

Choose a reason for hiding this comment

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

well, they should be careful if it goes higher (especially if it happens often). Note that 50 is quite arbitrary here. Blackfire may turn the corresponding metric into red on a different threshold for instance.
the general rule is that lots of DB queries is a bad sign for perf (because DB queries take time)

Copy link
Member

Choose a reason for hiding this comment

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

I like the clarifiation, but I think it's a bit too long. What do you think about:

The icon will turn yellow if there were more than 50 queries on the page, this could indicate that something is not correct.

Copy link
Author

Choose a reason for hiding this comment

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

Sure, sounds great to me. I was mainly concerned with getting the information about the yellow icon in there.

@xabbuh
Copy link
Member

xabbuh commented Mar 8, 2015

@XitasoChris Can you apply the change suggested by @wouterj?

@ghost
Copy link
Author

ghost commented Mar 23, 2015

@xabbuh Done, sorry for the delay.

@xabbuh
Copy link
Member

xabbuh commented Mar 23, 2015

@XitasoChris Thanks 👍

@weaverryan
Copy link
Member

This is a great PR - something wasn't clear, so you've cleared it up for others. Thanks so much for this @XitasoChris!

weaverryan added a commit that referenced this pull request Mar 24, 2015
…oChris)

This PR was submitted for the 2.6 branch but it was merged into the 2.3 branch instead (closes #4842).

Discussion
----------

Add meaning of yellow icon for number of queries

Some people (me included) have been confused about why the number of queries is sometimes displayed in a yellow color. Added explanation for this.

Number is from https://stackoverflow.com/questions/17859176/what-does-the-yellow-color-warning-icon-mean-in-symfony2-web-profiler

Commits
-------

5e687b2 changes as suggested by WouterJ in pull request #4842
5ce0f3d Add meaning of yellow icon for number of queries
weaverryan added a commit that referenced this pull request Mar 24, 2015
@weaverryan weaverryan closed this Mar 24, 2015
weaverryan added a commit that referenced this pull request Mar 24, 2015
* 2.3: (44 commits)
  Removed duplicate "long"s
  terminate file with newline
  move title back and move description to separate file as suggested in PR #4892
  move title out of included file as suggested in PR #4892
  Add possible values for widget_type
  [#4842] Making 2 sentences
  changes as suggested by WouterJ in pull request #4842
  Add meaning of yellow icon for number of queries
  Fixing bad link name
  fix typo in event flow diagrams
  Many fixes thanks to stof, WouterJ, xabbuh and dupuchba
  added Jakub as a merger for the DomCrawler component
  [#5094] Removing mkdir - it's not needed (thanks xabbuh)
  some tweaks to #4601
  Moving index down to correct section
  [#4989] Language tweaks and making the example simpler
  Remove useless setLocale() call and add code block with locale setter
  Finaly touches on translation locale setting note
  Review note about setting the translator locale in a controller.
  Update translation.rst
  ...

Conflicts:
	cookbook/security/entity_provider.rst
weaverryan added a commit that referenced this pull request Mar 24, 2015
* 2.6: (45 commits)
  Removed duplicate "long"s
  terminate file with newline
  move title back and move description to separate file as suggested in PR #4892
  move title out of included file as suggested in PR #4892
  Add possible values for widget_type
  [#4842] Making 2 sentences
  changes as suggested by WouterJ in pull request #4842
  Add meaning of yellow icon for number of queries
  Fixing bad link name
  fix typo in event flow diagrams
  Many fixes thanks to stof, WouterJ, xabbuh and dupuchba
  added Jakub as a merger for the DomCrawler component
  [#5094] Removing mkdir - it's not needed (thanks xabbuh)
  some tweaks to #4601
  Moving index down to correct section
  [#4989] Language tweaks and making the example simpler
  Remove useless setLocale() call and add code block with locale setter
  Finaly touches on translation locale setting note
  Review note about setting the translator locale in a controller.
  Update translation.rst
  ...
weaverryan added a commit that referenced this pull request Mar 24, 2015
* 2.7: (47 commits)
  Removed duplicate "long"s
  terminate file with newline
  move title back and move description to separate file as suggested in PR #4892
  move title out of included file as suggested in PR #4892
  Add possible values for widget_type
  [#4842] Making 2 sentences
  changes as suggested by WouterJ in pull request #4842
  Add meaning of yellow icon for number of queries
  Fixing bad link name
  fix typo in event flow diagrams
  Many fixes thanks to stof, WouterJ, xabbuh and dupuchba
  added Jakub as a merger for the DomCrawler component
  Changes thanks to WouterJ and xabbuh
  [#5094] Removing mkdir - it's not needed (thanks xabbuh)
  some tweaks to #4601
  Moving index down to correct section
  [#4989] Language tweaks and making the example simpler
  Remove useless setLocale() call and add code block with locale setter
  Finaly touches on translation locale setting note
  Review note about setting the translator locale in a controller.
  ...
@ghost ghost deleted the patch-1 branch March 24, 2015 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants