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

Rename JsonTracer as ChromeTracer #1183

Open
dayo09 opened this issue Aug 5, 2022 · 10 comments
Open

Rename JsonTracer as ChromeTracer #1183

dayo09 opened this issue Aug 5, 2022 · 10 comments

Comments

@dayo09
Copy link
Contributor

dayo09 commented Aug 5, 2022

What?

(just a suggestion, not about this PR)

It seems the name 'JsonTracer' is a bit confusing because we have another 'JsonTracer' module : mondrianViewer. Mondrian files are another type of our 'json' file which 'traces' mondrian (its file extension names are 'tracealloc.json').

Therefore, I suggest changing the module name as 'ChromeTracer'. We actually uses the 'chrome tracer' open source solution here, so the naming sounds more direct.

Originally posted by @dayo09 in #1172 (comment)

@llFreetimell
Copy link
Contributor

I agree with the background of this suggestion and also agree with the confusion :)

Only one thing I worry about is... "Can we really use the name of Chrome or ChromeTracer for our tool?" in legally.
Since Chrome and ChromeTracer is a kind of trademark of Google's, we may invade their right of the naming :(

So how about naming with another idea?
OneTracer, CycleViewer, TimelineViewer, etc.... nothing good idea comes now T.T

@dayo09
Copy link
Contributor Author

dayo09 commented Aug 8, 2022

@llFreetimell

We actually uses the 'chrome tracer' open source solution here, so the naming sounds more direct.

Well how do you think about the line I wrote above..?
We actually vulcanized their open source into our project, if we alter the name, then it may invade their efforts.

@llFreetimell
Copy link
Contributor

We actually vulcanized their open source into our project, so I assumed that altering the name invades their efforts.

If we embedded all or some part of ChromeTracer, I agree with you :)

But as JsonTracer is sort of copycat, which only referenced overall result of Chrome Tracer, I don't think this is sort of ChromeTracer. (Actually, JsonTracer did not copy source codes in ChromeTracer)

In addition, if we name it as ChromeTracer, people may misunderstand that we fully supports ChromeTracer features in ONE-vscode.
Or our own features which are not in ChromeTracer may be implemented in JsonTracer in the future.

But I totally agree that we need another name for Jsontracer...!

@dayo09
Copy link
Contributor Author

dayo09 commented Aug 8, 2022

@llFreetimell I thought the codes are from the catapult-project's trace viewer module..? https://github.com/catapult-project/catapult/blob/main/tracing/README.md

I don't know the history but so I want to ask: I think I heard about the project that it has ported chrome tracer viewer's part but I still remember that you said the authors has built it from scratch in javascript. If so, I agree that it doesn't need to be chrome tracer.

@dayo09
Copy link
Contributor Author

dayo09 commented Aug 8, 2022

But we should keep in mind that our trace generation command is --save-chrome-trace. And that our trace file's json formatting is

"displayTimeUnit": "ns",
"traceEvents": [

This formatting and data structure actually is 'chrome trace' format.

So to me, it sounds weird to say that we shouldn't use 'Chrome Tracer' word because it "invades their right of naming." - as you said.

@dayo09
Copy link
Contributor Author

dayo09 commented Aug 8, 2022

Let's list up all the candidates then.

  • Json Tracer

  • Json Trace Viewer

  • Trace Viewer

  • Chrome Trace Viewer

  • Latency Trace Viewer

  • Performance Trace Viewer

  • Performance Viewer

  • (more)

IMO,

  • "Json Tracer, Json Trace Viewer, Trace Viewer" sounds confusing along with mondrian, which uses the extension *.tracealloc.json.
  • " Chrome Trace Viewer" sounds fine (Personal pick 😃) but with some objection about using 'chrome' naming.
  • "Latency Trace Viewer", "Performance Trace Viewer", "Performance Viewer" may be another alternatives.

@ejjeong Could you give your opinion, too? It will be very helpful :-D
FYI, JsonTracer is our visualization module of chrome trace files. Find this gif to get the basic idea :-D.

@dayo09 dayo09 added this to the Sprint (Aug 15 to Aug 26) milestone Aug 16, 2022
@ejjeong
Copy link

ejjeong commented Aug 29, 2022

@ejjeong Could you give your opinion, too? It will be very helpful :-D

Ah, sorry I missed this comment 😢
I turned off the notifications from this repo (just because it is very active 😄).

I don't have any strong preference among the current candidates,
especially because I'm not aware of which "open source project" JsonTracer is based on.

Anyway, I'd like to add another candidate: "trace event viewer"
The backend profiler generates the trace in Trace Event Format.

@ejjeong
Copy link

ejjeong commented Aug 29, 2022

BTW, just out of curiosity, was there any reason that JsonTracer did not use the catapult Trace-Viewer?
It even provides trace2html, whose output should be easy to be shown in the vscode.

Ah, maybe is it because it's hard to run python code in vscode extentions?

@dayo09
Copy link
Contributor Author

dayo09 commented Sep 15, 2022

Anyway, I'd like to add another candidate: "trace event viewer"

Thanks for giving your opinion! :-D

BTW, just out of curiosity, was there any reason that JsonTracer did not use the catapult Trace-Viewer?

@llFreetimell Maybe he knows..?

@llFreetimell
Copy link
Contributor

llFreetimell commented Sep 15, 2022

Sorry for missing this issue... :(

I am confused now because I regared Trace-Viewer as Chrome Tracer....
Maybe there are something I missed... T.T
So I want to withdraw my suggestions and objections 😅

was there any reason that JsonTracer did not use the catapult Trace-Viewer?

JsonTracer referenced Trace-Viewer, which I called as Chrome Tracer... T.T

// This file referenced the result of
// https://github.com/catapult-project/catapult/tree/444aba89e1c30edf348c611a9df79e2376178ba8/tracing

And at that time, embedding the full source codes into here requires much time to validate the license but there are not much time for us..!
I think we can consider it now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants