Page MenuHomeWildfire Games

Rewrite Profiler2's GUI using D3 / CanvasJS
AbandonedPublic

Authored by wraitii on Jan 27 2019, 1:55 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

This rewrites Profiler2's UI to use D3 and CanvasJS instead of custom functions.
Cons:

  • Somewhat slower for the timeline in D3 which now uses svg elements, but it seems OK in the general case.
  • Dependency on canvasJS / D3
  • requires a locally-running http server (see run.sh)
  • Basically guaranteed to only work in Chrome but I think the former version did that already.

Pros:

  • much less code which makes this actually upgradable. I've also cleaned up things.
  • more interactivity: clicking on stuff does things and zooming is more convenient.
  • much easier to expand.

I need to add back support for "Live" profiling, but tbh that's not the most useful feature and I might just kill it.

Test Plan

Open a profile. Play around with stuff.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
__profiler2rewrite
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 8898
Build 14604: Vulcan BuildJenkins
Build 14603: arc lint + arc unit

Event Timeline

wraitii created this revision.Jan 27 2019, 1:55 PM
Vulcan added a subscriber: Vulcan.Jan 27 2019, 1:59 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1005/

Stan added a subscriber: Stan.Jan 27 2019, 2:22 PM
Stan added inline comments.
source/tools/profiler2/ReportColor.js
24

Should be g_something, with maybe a comment :)

29

I believe the convention is

!(color_by_name.name)

source/tools/profiler2/index.html
11

Can you make a css file, this is bad :D

Stan added a subscriber: elexis.Jan 27 2019, 2:25 PM
Stan added inline comments.
source/tools/profiler2/Profiler2Report.js
35

@elexis Do we use this anywhere else ? It looks nicer than prototypes. If not maybe we should.

Stan added inline comments.Jan 27 2019, 2:32 PM
source/tools/profiler2/Profiler2Report.js
59

Enum ?

nani added a subscriber: nani.Jan 27 2019, 2:33 PM
nani added inline comments.
source/tools/profiler2/Profiler2Report.js
35

The current spider-monkey in 0ad doesn't have support for classes afaik

Stan added inline comments.Jan 27 2019, 2:35 PM
source/tools/profiler2/Profiler2Report.js
24–25

Missing 'use strict'; (It's by default in SpiderMonkey code but not here I guess"

35

That's what I thought.

lyv added a subscriber: lyv.Jan 27 2019, 2:39 PM
lyv added inline comments.
source/tools/profiler2/ReportColor.js
27

export {get_color, default} at the bottom would be nicer no?

29

Should be using [].

lyv added inline comments.Jan 27 2019, 2:41 PM
source/tools/profiler2/Profiler2Report.js
35

This came with es6 which means the current 0AD SM does have it. But it's just syntactic sugar over the prototype system.

lyv removed a subscriber: lyv.Jan 27 2019, 2:43 PM
wraitii updated this revision to Diff 9372.Aug 18 2019, 12:42 PM
wraitii retitled this revision from Rewrite Profiler2's GUI using highcharts to Rewrite Profiler2's GUI using D3 / CanvasJS.
wraitii edited the summary of this revision. (Show Details)

Do it in CanvasJS and D3 instead, Highcharts was too slow.

License needs some sorting out but I think we're in the clear.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/408/display/redirect

Krinkle added inline comments.
source/tools/profiler2/index.html
8

Use explicit HTTPS if it is supported :) More secure (never tolerate HTTP) and has the benefit of also working when on urls with the file-scheme (I don't know if that be the case here).

elexis added inline comments.Aug 18 2019, 4:31 PM
source/tools/profiler2/index.html
8

Is it right to include code from other pags like that? I know its standard on the WWW, but this page isn't part of it and only localhost?
(Also would github happy about hotlinking? https://github.com/moment/momentjs.com/issues/68)

It also wouldnt prevent running the profiler2 without being connected to the internet.
On the other side distributing lots of source code is unfortunate too.

wraitii abandoned this revision.Jun 3 2023, 11:05 AM

Not worth doing as-is