Bug 31784 - Web Inspector: Reimplement TimelinePanel to make it scale
Summary: Web Inspector: Reimplement TimelinePanel to make it scale
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-22 04:32 PST by Pavel Feldman
Modified: 2009-11-22 10:25 PST (History)
4 users (show)

See Also:


Attachments
[PATCH] the patch. (64.35 KB, patch)
2009-11-22 04:35 PST, Pavel Feldman
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2009-11-22 04:32:37 PST
Today instrumenting 30 seconds of cnn.com or techcrunch.com makes timeline unresponsive. The reason is that several thousands or recurds result in proportional number of elements attached to the dom.

What this change does:
- Extracts grid and overview classes into separate files
- Makes timeline create only divs for visible rows
- It makes timeline really fast and scalable. We can further optimize it via bucketing events by time if that is needed (like in order to handle >100K entries).
Comment 1 Pavel Feldman 2009-11-22 04:35:53 PST
Created attachment 43675 [details]
[PATCH] the patch.
Comment 2 Timothy Hatcher 2009-11-22 07:59:30 PST
Comment on attachment 43675 [details]
[PATCH] the patch.

> +        var visibleTop = this._containerElement.scrollTop;
> +        var visibleBottom = visibleTop + this._containerElement.clientHeight;
> +        var rowHeight = 18;
> +        var startIndex = Math.max(0, Math.floor(visibleTop / rowHeight) - 1);
> +        var endIndex = Math.min(recordsInWindow.length, Math.ceil(visibleBottom / rowHeight));

Use const instead of var for these.


> +        var top = (startIndex * rowHeight) + "px";

Use const.


>  .sidebar {
>      position: absolute;
>      top: 0;
> +    min-height: 100%;
>      left: 0;
> -    bottom: 0;

Why was this needed?


>  .sidebar-resizer-vertical {
>      position: absolute;
>      top: 0;
> -    bottom: 0;
> +    min-height: 100%;

Ditto.


> +.timeline-tree-section {

Is this class uses in your patch? What is this for?


> +    color: rgb(92, 110, 129);
> +    font-family: Helvetica, Arial, sans-serif;

Why is this Helvetica? We don't use Helvetica except in a few rare places. Otherwise the font is inherited from the body.


> +    position: relative;

Why relative, so it have positioned children?


> +    text-shadow: rgba(255, 255, 255, 0.746094) 0px 1px 0px;

Just use 0.75 instead of 0.746094. The CSS parser ignores the extra precision anyway.


> +    font-family: Helvetica, Arial, sans-serif;

Why specify the font?
Comment 3 Pavel Feldman 2009-11-22 10:17:12 PST
> Use const instead of var for these.
> 

Done.

> 
> Use const.
> 

Done.

> 
> >  .sidebar {
> >      position: absolute;
> >      top: 0;
> > +    min-height: 100%;
> >      left: 0;
> > -    bottom: 0;
> 
> Why was this needed?
> 

I shared an image on IRC.

> 
> > +.timeline-tree-section {
> 
> Is this class uses in your patch? What is this for?
> 

Should not be here - from a previous version of patch. Removed.


> Why is this Helvetica? We don't use Helvetica except in a few rare places.
> Otherwise the font is inherited from the body.
> Why relative, so it have positioned children?

I was experimenting and forgot to delete. from a previous version of patch. Removed.


> > +    text-shadow: rgba(255, 255, 255, 0.746094) 0px 1px 0px;
> 
> Just use 0.75 instead of 0.746094. The CSS parser ignores the extra precision
> anyway.
> 

That was a copypaste from inspector. Fixed.
Comment 4 Pavel Feldman 2009-11-22 10:25:24 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/inspector/timeline-test.js
	M	WebCore/ChangeLog
	M	WebCore/WebCore.gypi
	M	WebCore/WebCore.vcproj/WebCore.vcproj
	M	WebCore/inspector/front-end/AbstractTimelinePanel.js
	A	WebCore/inspector/front-end/TimelineGrid.js
	A	WebCore/inspector/front-end/TimelineOverviewPane.js
	M	WebCore/inspector/front-end/TimelinePanel.js
	M	WebCore/inspector/front-end/WebKit.qrc
	M	WebCore/inspector/front-end/inspector.css
	M	WebCore/inspector/front-end/inspector.html
Committed r51296