ONE-vscode icon indicating copy to clipboard operation
ONE-vscode copied to clipboard

Rename JsonTracer as ChromeTracer

Open dayo09 opened this issue 2 years ago • 10 comments

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 https://github.com/Samsung/ONE-vscode/pull/1172#discussion_r938619285

dayo09 avatar Aug 05 '22 09:08 dayo09

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

llFreetimell avatar Aug 05 '22 11:08 llFreetimell

@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.

dayo09 avatar Aug 08 '22 05:08 dayo09

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...!

llFreetimell avatar Aug 08 '22 06:08 llFreetimell

@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 avatar Aug 08 '22 09:08 dayo09

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 avatar Aug 08 '22 10:08 dayo09

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 avatar Aug 08 '22 10:08 dayo09

@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 avatar Aug 29 '22 02:08 ejjeong

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?

ejjeong avatar Aug 29 '22 02:08 ejjeong

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..?

dayo09 avatar Sep 15 '22 06:09 dayo09

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 https://github.com/Samsung/ONE-vscode/blob/727a52a776478fe373996a64587bb50a89999523/media/Jsontracer/bar.js#L45-L46

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 :)

llFreetimell avatar Sep 15 '22 07:09 llFreetimell