search

Samsung / ONE-vscode

Visual Studio Code Extension of ONE compiler toolchain
Apache License 2.0
48 stars 50 forks source link

Rename JsonTracer as ChromeTracer #1183

Open dayo09 opened 2 years ago

dayo09 commented 2 years ago

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_

llFreetimell commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

Let's list up all the candidates then.

IMO,

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

ejjeong commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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