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

[Profiler] Render the performance graph with SVG #30450

Merged
merged 1 commit into from Mar 17, 2019

Conversation

Projects
None yet
8 participants
@Tom32i
Copy link
Contributor

commented Mar 5, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets Part 1, 3 and 4 of #27262
License MIT
Doc PR n/a

Following a suggestion by @ogizanagi in #27262,
Here's a proposal to render the Request Graph, from the performance profiler panel, with SVG instead of canvas.

Some benefits of the SVG format:

  • The text labels are searchable and can be selected.
  • It renders well on high DPI monitors.
  • Colors and text styles can be defined with CSS just like the rest of the page.

In addition, SVG allow us to consider (and easily implement) interactives features such as:

  • Zoom in and time navigation (thanks to the viewport).
  • Highlight hovered line (or other DOM related events).

Preview:

screenshot_2019-03-08 symfony profiler 1

Filtered events example:

capture d ecran 2019-03-08 a 17 22 47

Progress :

  • Render request events in SVG
  • Show labels with duration and memory
  • Show specific markers at start / end of lines
  • Re-render graph when window resize
  • Re-render graph when threshold change.
  • Generate graph legend with only existing categories (part 1. of #27262 )
  • Show sub-request area with hatched pattern
  • Allow to hide categories by clicking them on the legend (part 3. of #27262 )
  • Handle text overflow on long labels.
  • Ensure JS code is compatible with all supported browsers (used classes and arrow functions.
  • Add left-padding to sub-request graph?

@Tom32i Tom32i changed the title [Profiler] Render the performances graph with SVG [Profiler] Render the performance graph with SVG Mar 5, 2019

@Tom32i Tom32i force-pushed the Tom32i:feature/web-profiler/svg-graph branch from 2cfeed2 to 6ed4a19 Mar 5, 2019

@xabbuh xabbuh added this to the next milestone Mar 5, 2019

@Tom32i Tom32i force-pushed the Tom32i:feature/web-profiler/svg-graph branch from df757c9 to 7b76f5b Mar 6, 2019

@Tom32i Tom32i force-pushed the Tom32i:feature/web-profiler/svg-graph branch from a9ea811 to a2d016a Mar 7, 2019

@Tom32i Tom32i force-pushed the Tom32i:feature/web-profiler/svg-graph branch 3 times, most recently from 588f68f to 362e12f Mar 12, 2019

@Tom32i Tom32i marked this pull request as ready for review Mar 13, 2019

@Tom32i

This comment has been minimized.

Copy link
Contributor Author

commented Mar 13, 2019

Status: Needs Review

@fabpot

fabpot approved these changes Mar 16, 2019

@fabpot fabpot force-pushed the Tom32i:feature/web-profiler/svg-graph branch from 6a1a7b3 to a69a718 Mar 17, 2019

@fabpot

This comment has been minimized.

Copy link
Member

commented Mar 17, 2019

Thank you @Tom32i.

@fabpot fabpot merged commit a69a718 into symfony:master Mar 17, 2019

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

fabpot added a commit that referenced this pull request Mar 17, 2019

feature #30450 [Profiler] Render the performance graph with SVG (Tom32i)
This PR was squashed before being merged into the 4.3-dev branch (closes #30450).

Discussion
----------

[Profiler] Render the performance graph with SVG

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | Part 1, 3 and 4 of #27262
| License       | MIT
| Doc PR        | n/a

Following a suggestion by @ogizanagi in #27262,
Here's a proposal to render the Request Graph, from the performance profiler panel, with SVG instead of canvas.

Some benefits of the SVG format:
- The text labels are searchable and can be selected.
- It renders well on high DPI monitors.
- [Colors and text styles](#27262 (comment)) can be defined with CSS just like the rest of the page.

In addition, SVG allow us to consider (and easily implement) interactives features such as:
- Zoom in and time navigation (thanks to the viewport).
- Highlight hovered line (or other DOM related events).

Preview:

![screenshot_2019-03-08 symfony profiler 1](https://user-images.githubusercontent.com/1846873/54036727-a33f4300-41bc-11e9-8be7-b1de10d4afd9.png)

Filtered events example:

![capture d ecran 2019-03-08 a 17 22 47](https://user-images.githubusercontent.com/1846873/54041039-00d88d00-41c7-11e9-9590-23e809415c34.png)

### Progress :
- [x] Render request events in SVG
- [x] Show labels with duration and memory
- [x] Show specific markers at start / end of lines
- [x] Re-render graph when window resize
- [x] Re-render graph when threshold change.
- [x]  Generate graph legend with only existing categories (part 1. of #27262 )
- [x] Show sub-request area with hatched pattern
- [x]  Allow to hide categories by clicking them on the legend (part 3. of #27262 )
- [x] Handle text overflow on long labels.
- [x] Ensure JS code is compatible with all supported browsers (used [classes](https://caniuse.com/#feat=es6-class) and [arrow functions](https://caniuse.com/#feat=arrow-functions).
- ~Add left-padding to sub-request graph?~

Commits
-------

a69a718 [Profiler] Render the performance graph with SVG
@staabm

This comment has been minimized.

Copy link
Contributor

commented Mar 17, 2019

Any chance to get this element as a standalone component, so we can re-use it in other web-frontends?

@staabm staabm referenced this pull request Mar 17, 2019

Open

Performance element #18

fabpot added a commit that referenced this pull request Apr 6, 2019

bug #30937 [Profiler] Fix graph text color (dFayet)
This PR was merged into the 4.3-dev branch.

Discussion
----------

[Profiler] Fix graph text color

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | -   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | -

#30450 introduces bad text color for graph items in the profiler, specially in dark theme mode.

### before/after

![Capture d’écran de 2019-04-06 21-38-29](https://user-images.githubusercontent.com/7721219/55674505-ff79ad80-58b5-11e9-9a29-8359ff07673d.png)

![Capture d’écran de 2019-04-06 21-42-26](https://user-images.githubusercontent.com/7721219/55674507-030d3480-58b6-11e9-8dee-395e2046e7ca.png)

There is a small change for default theme

### before/after

![Capture d’écran de 2019-04-06 21-51-46](https://user-images.githubusercontent.com/7721219/55674538-4ff10b00-58b6-11e9-8aed-2621bdde673d.png)

![Capture d’écran de 2019-04-06 21-52-15](https://user-images.githubusercontent.com/7721219/55674540-52ebfb80-58b6-11e9-9d94-68743187aba2.png)

<!--
Write a short README entry for your feature/bugfix here (replace this comment block.)
This will help people understand your PR and can be used as a start of the Doc PR.
Additionally:
 - Bug fixes must be submitted against the lowest branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the master branch.
-->

<!--
Write a short README entry for your feature/bugfix here (replace this comment block.)
This will help people understand your PR and can be used as a start of the Doc PR.
Additionally:
 - Bug fixes must be submitted against the lowest branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the master branch.
-->

Commits
-------

44d9fbe Fix graph text color

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019

@fabpot fabpot referenced this pull request May 9, 2019

Merged

Release v4.3.0-BETA1 #31435

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.