Skip to content

[wasm-ep] Minimal diagnostic tracing configuration and sample - #69158

Merged
lambdageek merged 38 commits into
dotnet:mainfrom
lambdageek:wasm-ep
May 19, 2022
Merged

[wasm-ep] Minimal diagnostic tracing configuration and sample#69158
lambdageek merged 38 commits into
dotnet:mainfrom
lambdageek:wasm-ep

Conversation

@lambdageek

@lambdageeklambdageek commented May 10, 2022

Copy link
Copy Markdown
Member

  • Adds a /p:WasmEnablePerfTracing=true configuration.

    In this configuration the runtime is built with threading (MonoWasmThreads property is true), but C# code is not allowed to start threads and doesn't use the portable thradpool (MonoWasmThreadsNoUser property is also true).

    The upshot is that EventPipe can start threads but user code is still single-threaded.

  • Adds a MONO.diagnostics interface in JavaScript. There's a single method for now createEventPipeSession which creates a session that can save a trace to a file on the virtual file system. JS code (or a user in a dev console) needs to call start() and stop() on the session object to begin collecting samples. For now there's no way to specify any configuration options other than the destination VFS file path for the trace file.

  • Adds a sample that runs an async task for five seconds and collects samples and then triggers a click() to download the trace file out of the browser.

@ghostghost assigned lambdageekMay 10, 2022
@ghost

ghost commented May 10, 2022

Copy link
Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

  • Adds a /p:WasmEnablePerfTracing=true configuration.

    In this configuration the runtime is built with threading (MonoWasmThreads property is true), but C# code is not allowed to start threads and doesn't use the portable thradpool (MonoWasmThreadsNoUser property is also true).

    The upshot is that EventPipe can start threads but user code is still single-threaded.

  • Adds a MONO.diagnostics interface in JavaScript. There's a single method for now createEventPipeSession which creates a session that can save a trace to a file on the virtual file system. JS code (or a user in a dev console) needs to call start() and stop() on the session object to begin collecting samples. For now there's no way to specify any configuration options other than the destination VFS file path for the trace file.

  • Adds a sample that runs an async task for five seconds and collects samples and then triggers a click() to download the trace file out of the browser.

Author:lambdageek
Assignees:-
Labels:

arch-wasm, area-System.Diagnostics-mono

Milestone:-

@lambdageek

lambdageek commented May 10, 2022

Copy link
Copy Markdown
MemberAuthor

/cc @kg@lateralusX

@lambdageek

lambdageek commented May 10, 2022

Copy link
Copy Markdown
MemberAuthor

/cc @pavelsavara

@lambdageek
lambdageek marked this pull request as ready for review May 10, 2022 20:50
pavelsavara
pavelsavara reviewed May 10, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/types.ts Outdated
@lambdageek
don't run wasm-eventpipe sample on CI lanes without perftracing
e2e1b66
@lambdageek
more whitespace
ded3aeb
@lambdageek
fix eslint warnings, default rundown to true, allow callback for trac…
46c1a7c
…eFilePath option
@lambdageek
add EventPipeSession.getTraceBlob and EventPipeSession.getTraceDataURI
c29915d
two ways of retreiving the collected traces instead of exposing the
emscripten VFS directly.
Probably the Blob one is enough. Is there any reason to also provide
a data URI?
update the sample to use URL.createObjectURL (session.getTraceBlob())
to create the download link
lateralusX
lateralusX reviewed May 11, 2022
View reviewed changes

@lateralusXlateralusX left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EventPipe bits looks good, just a couple of small comments/thoughts.

Comment threadsrc/mono/mono/component/event_pipe-stub.c Outdated
Comment threadsrc/mono/mono/component/event_pipe-stub.c Outdated
@azure-pipelines

azure-pipelinesBot commented May 16, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@lambdageek

lambdageek commented May 17, 2022

Copy link
Copy Markdown
MemberAuthor

Any other feedback?
/cc @lewing

@lambdageek

lambdageek commented May 18, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

azure-pipelinesBot commented May 18, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

kg
kg reviewed May 18, 2022
View reviewed changes
Comment threadsrc/mono/mono/component/event_pipe.c
kg
kg reviewed May 18, 2022
View reviewed changes
Comment threadsrc/mono/mono/metadata/threads.c
kg
kg reviewed May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/Program.cs Outdated
kg
kg reviewed May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/index.html Outdated
kg
kg reviewed May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/main.js
globalThis.MONO = MONO;
console.log('after createDotnetRuntime')

const startWork = BINDING.bind_static_method("[Wasm.Browser.EventPipe.Sample] Sample.Test:StartAsyncWork");

@kgkgMay 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we rely on the non-Main methods in the sample maybe just put a single line comment in Program.cs's Main to indicate that the reader should look in main.js for the juicy bits

@lambdageeklambdageekMay 18, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

kg
kg reviewed May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/cuint64.ts
kg
kg reviewed May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/diagnostics.ts
kg
kg reviewed May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/memory.ts Outdated
}

export function getCU64(offset: _MemOffset): cuint64.CUInt64 {
const lo = getI32(offset);

@kgkgMay 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these actually be signed? It seems like they should be unsigned.

kg
kg reviewed May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/types/emscripten.ts
}

export declare type TypedArray = Int8Array | Uint8Array | Uint8ClampedArray | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array; No newline at end of file
export declare type TypedArray = Int8Array | Uint8Array | Uint8ClampedArray | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array;

@kgkgMay 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also use ArrayBufferView in place of this, though it has the downside of that interface inexplicably not having .length. Just an FYI, I'm fine with us defining a TypedArray alias (do the bigint arrays need to go in here too?)

@lambdageeklambdageekMay 18, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't change anything here. I think eslint just added or removed a newline.

kg
kg approved these changes May 18, 2022
View reviewed changes
@lambdageek

lambdageek commented May 18, 2022

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

azure-pipelinesBot commented May 18, 2022

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@lewing
lewing requested a review from radekdoulikMay 18, 2022 15:17
lewing
lewing approved these changes May 18, 2022
View reviewed changes

@lewinglewing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@lambdageek
lambdageek merged commit b15ff06 into dotnet:mainMay 19, 2022
@ghostghost locked as resolved and limited conversation to collaborators Jun 18, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@pavelsavarapavelsavarapavelsavara left review comments
@lateralusXlateralusXlateralusX left review comments
@lewinglewinglewing approved these changes
@thaystgthaystgthaystg approved these changes
@marek-safarmarek-safarAwaiting requested review from marek-safar
@radekdoulikradekdoulikAwaiting requested review from radekdoulik
+1 more reviewer
@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@lambdageeklambdageek

Labels

arch-wasmWebAssembly architecturearea-System.Diagnostics-mono

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@lambdageek@pavelsavara@lewing@kg@thaystg@lateralusX
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
[wasm-ep] Minimal diagnostic tracing configuration and sample by lambdageek · Pull Request #69158 · dotnet/runtime · GitHub
Skip to content
[wasm-ep] Minimal diagnostic tracing configuration and sample - #69158
Merged
lambdageek merged 38 commits into
dotnet:mainfrom 
lambdageek:wasm-ep
May 19, 2022
Merged
[wasm-ep] Minimal diagnostic tracing configuration and sample#69158
lambdageek merged 38 commits into
dotnet:mainfrom 
lambdageek:wasm-ep
Conversation
@lambdageek
@lambdageeklambdageek
commented
May 10, 2022

Copy link


Copy Markdown

Member
  • Adds a /p:WasmEnablePerfTracing=true configuration.
    In this configuration the runtime is built with threading (MonoWasmThreads property is true), but C# code is not allowed to start threads and doesn't use the portable thradpool (MonoWasmThreadsNoUser property is also true).
    The upshot is that EventPipe can start threads but user code is still single-threaded.
  • Adds a MONO.diagnostics interface in JavaScript. There's a single method for now createEventPipeSession which creates a session that can save a trace to a file on the virtual file system. JS code (or a user in a dev console) needs to call start() and stop() on the session object to begin collecting samples. For now there's no way to specify any configuration options other than the destination VFS file path for the trace file.
  • Adds a sample that runs an async task for five seconds and collects samples and then triggers a click() to download the trace file out of the browser.
@ghostghost
assigned lambdageekMay 10, 2022
@ghost
ghost
commented
May 10, 2022

Copy link


Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing

See info in area-owners.md if you want to be subscribed.
Issue Details
  • Adds a /p:WasmEnablePerfTracing=true configuration.
    In this configuration the runtime is built with threading (MonoWasmThreads property is true), but C# code is not allowed to start threads and doesn't use the portable thradpool (MonoWasmThreadsNoUser property is also true).
    The upshot is that EventPipe can start threads but user code is still single-threaded.
  • Adds a MONO.diagnostics interface in JavaScript. There's a single method for now createEventPipeSession which creates a session that can save a trace to a file on the virtual file system. JS code (or a user in a dev console) needs to call start() and stop() on the session object to begin collecting samples. For now there's no way to specify any configuration options other than the destination VFS file path for the trace file.
  • Adds a sample that runs an async task for five seconds and collects samples and then triggers a click() to download the trace file out of the browser.
Author:lambdageek
Assignees:-
Labels:
arch-wasm, area-System.Diagnostics-mono
Milestone:-
@lambdageek
lambdageek
commented
May 10, 2022

Copy link


Copy Markdown

MemberAuthor
/cc @kg@lateralusX
@lambdageek
lambdageek
commented
May 10, 2022

Copy link


Copy Markdown

MemberAuthor
/cc @pavelsavara
@lambdageek
lambdageek
marked this pull request as ready for review
May 10, 2022 20:50
pavelsavara
pavelsavara
reviewed
May 10, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/types.ts
Outdated

@lambdageek
don't run wasm-eventpipe sample on CI lanes without perftracing
e2e1b66
@lambdageek
more whitespace
ded3aeb
@lambdageek
fix eslint warnings, default rundown to true, allow callback for trac…
46c1a7c
…eFilePath option
@lambdageek
add EventPipeSession.getTraceBlob and EventPipeSession.getTraceDataURI
c29915d
two ways of retreiving the collected traces instead of exposing the
emscripten VFS directly.
Probably the Blob one is enough. Is there any reason to also provide
a data URI?
update the sample to use URL.createObjectURL (session.getTraceBlob())
to create the download link
lateralusX
lateralusX
reviewed
May 11, 2022
View reviewed changes
@lateralusXlateralusX
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EventPipe bits looks good, just a couple of small comments/thoughts.
Comment threadsrc/mono/mono/component/event_pipe-stub.c
Outdated

Comment threadsrc/mono/mono/component/event_pipe-stub.c
Outdated

@azure-pipelines
azure-pipelinesBot
commented
May 16, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@lambdageek
lambdageek
commented
May 17, 2022

Copy link


Copy Markdown

MemberAuthor
Any other feedback?

/cc @lewing
@lambdageek
lambdageek
commented
May 18, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
May 18, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/mono/component/event_pipe.c
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/mono/metadata/threads.c
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/Program.cs
Outdated

kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/index.html
Outdated

kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/main.js
 globalThis.MONO = MONO;
 console.log('after createDotnetRuntime')

 const startWork = BINDING.bind_static_method("[Wasm.Browser.EventPipe.Sample] Sample.Test:StartAsyncWork");
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we rely on the non-Main methods in the sample maybe just put a single line comment in Program.cs's Main to indicate that the reader should look in main.js for the juicy bits
@lambdageeklambdageekMay 18, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/cuint64.ts
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/diagnostics.ts
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/memory.ts
Outdated

}

export function getCU64(offset: _MemOffset): cuint64.CUInt64 {
 const lo = getI32(offset);
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these actually be signed? It seems like they should be unsigned.
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/types/emscripten.ts
}

export declare type TypedArray = Int8Array | Uint8Array | Uint8ClampedArray | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array; No newline at end of file 
export declare type TypedArray = Int8Array | Uint8Array | Uint8ClampedArray | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array;
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also use ArrayBufferView in place of this, though it has the downside of that interface inexplicably not having .length. Just an FYI, I'm fine with us defining a TypedArray alias (do the bigint arrays need to go in here too?)
@lambdageeklambdageekMay 18, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't change anything here. I think eslint just added or removed a newline.
kg
kg
approved these changes
May 18, 2022
View reviewed changes
@lambdageek
lambdageek
commented
May 18, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
May 18, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@lewing
lewing
requested a review
from radekdoulikMay 18, 2022 15:17
lewing
lewing
approved these changes
May 18, 2022
View reviewed changes
@lewinglewing
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:
@lambdageek
lambdageek
merged commit b15ff06
into
dotnet:mainMay 19, 2022
@ghostghost
locked as resolved and limited conversation to collaborators
Jun 18, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@pavelsavarapavelsavarapavelsavara left review comments
@lateralusXlateralusXlateralusX left review comments
@lewinglewinglewing approved these changes
@thaystgthaystgthaystg approved these changes
@marek-safarmarek-safarAwaiting requested review from marek-safar
@radekdoulikradekdoulikAwaiting requested review from radekdoulik

+1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@lambdageeklambdageek

Labels

arch-wasmWebAssembly architecturearea-System.Diagnostics-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@lambdageek@pavelsavara@lewing@kg@thaystg@lateralusX

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
[wasm-ep] Minimal diagnostic tracing configuration and sample by lambdageek · Pull Request #69158 · dotnet/runtime · GitHub
Skip to content
[wasm-ep] Minimal diagnostic tracing configuration and sample - #69158
Merged
lambdageek merged 38 commits into
dotnet:mainfrom 
lambdageek:wasm-ep
May 19, 2022
Merged
[wasm-ep] Minimal diagnostic tracing configuration and sample#69158
lambdageek merged 38 commits into
dotnet:mainfrom 
lambdageek:wasm-ep
Conversation
@lambdageek
@lambdageeklambdageek
commented
May 10, 2022

Copy link


Copy Markdown

Member
  • Adds a /p:WasmEnablePerfTracing=true configuration.
    In this configuration the runtime is built with threading (MonoWasmThreads property is true), but C# code is not allowed to start threads and doesn't use the portable thradpool (MonoWasmThreadsNoUser property is also true).
    The upshot is that EventPipe can start threads but user code is still single-threaded.
  • Adds a MONO.diagnostics interface in JavaScript. There's a single method for now createEventPipeSession which creates a session that can save a trace to a file on the virtual file system. JS code (or a user in a dev console) needs to call start() and stop() on the session object to begin collecting samples. For now there's no way to specify any configuration options other than the destination VFS file path for the trace file.
  • Adds a sample that runs an async task for five seconds and collects samples and then triggers a click() to download the trace file out of the browser.
@ghostghost
assigned lambdageekMay 10, 2022
@ghost
ghost
commented
May 10, 2022

Copy link


Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing

See info in area-owners.md if you want to be subscribed.
Issue Details
  • Adds a /p:WasmEnablePerfTracing=true configuration.
    In this configuration the runtime is built with threading (MonoWasmThreads property is true), but C# code is not allowed to start threads and doesn't use the portable thradpool (MonoWasmThreadsNoUser property is also true).
    The upshot is that EventPipe can start threads but user code is still single-threaded.
  • Adds a MONO.diagnostics interface in JavaScript. There's a single method for now createEventPipeSession which creates a session that can save a trace to a file on the virtual file system. JS code (or a user in a dev console) needs to call start() and stop() on the session object to begin collecting samples. For now there's no way to specify any configuration options other than the destination VFS file path for the trace file.
  • Adds a sample that runs an async task for five seconds and collects samples and then triggers a click() to download the trace file out of the browser.
Author:lambdageek
Assignees:-
Labels:
arch-wasm, area-System.Diagnostics-mono
Milestone:-
@lambdageek
lambdageek
commented
May 10, 2022

Copy link


Copy Markdown

MemberAuthor
/cc @kg@lateralusX
@lambdageek
lambdageek
commented
May 10, 2022

Copy link


Copy Markdown

MemberAuthor
/cc @pavelsavara
@lambdageek
lambdageek
marked this pull request as ready for review
May 10, 2022 20:50
pavelsavara
pavelsavara
reviewed
May 10, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/types.ts
Outdated

@lambdageek
don't run wasm-eventpipe sample on CI lanes without perftracing
e2e1b66
@lambdageek
more whitespace
ded3aeb
@lambdageek
fix eslint warnings, default rundown to true, allow callback for trac…
46c1a7c
…eFilePath option
@lambdageek
add EventPipeSession.getTraceBlob and EventPipeSession.getTraceDataURI
c29915d
two ways of retreiving the collected traces instead of exposing the
emscripten VFS directly.
Probably the Blob one is enough. Is there any reason to also provide
a data URI?
update the sample to use URL.createObjectURL (session.getTraceBlob())
to create the download link
lateralusX
lateralusX
reviewed
May 11, 2022
View reviewed changes
@lateralusXlateralusX
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EventPipe bits looks good, just a couple of small comments/thoughts.
Comment threadsrc/mono/mono/component/event_pipe-stub.c
Outdated

Comment threadsrc/mono/mono/component/event_pipe-stub.c
Outdated

@azure-pipelines
azure-pipelinesBot
commented
May 16, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@lambdageek
lambdageek
commented
May 17, 2022

Copy link


Copy Markdown

MemberAuthor
Any other feedback?

/cc @lewing
@lambdageek
lambdageek
commented
May 18, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
May 18, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/mono/component/event_pipe.c
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/mono/metadata/threads.c
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/Program.cs
Outdated

kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/index.html
Outdated

kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/main.js
 globalThis.MONO = MONO;
 console.log('after createDotnetRuntime')

 const startWork = BINDING.bind_static_method("[Wasm.Browser.EventPipe.Sample] Sample.Test:StartAsyncWork");
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we rely on the non-Main methods in the sample maybe just put a single line comment in Program.cs's Main to indicate that the reader should look in main.js for the juicy bits
@lambdageeklambdageekMay 18, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/cuint64.ts
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/diagnostics.ts
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/memory.ts
Outdated

}

export function getCU64(offset: _MemOffset): cuint64.CUInt64 {
 const lo = getI32(offset);
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these actually be signed? It seems like they should be unsigned.
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/types/emscripten.ts
}

export declare type TypedArray = Int8Array | Uint8Array | Uint8ClampedArray | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array; No newline at end of file 
export declare type TypedArray = Int8Array | Uint8Array | Uint8ClampedArray | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array;
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also use ArrayBufferView in place of this, though it has the downside of that interface inexplicably not having .length. Just an FYI, I'm fine with us defining a TypedArray alias (do the bigint arrays need to go in here too?)
@lambdageeklambdageekMay 18, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't change anything here. I think eslint just added or removed a newline.
kg
kg
approved these changes
May 18, 2022
View reviewed changes
@lambdageek
lambdageek
commented
May 18, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
May 18, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@lewing
lewing
requested a review
from radekdoulikMay 18, 2022 15:17
lewing
lewing
approved these changes
May 18, 2022
View reviewed changes
@lewinglewing
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:
@lambdageek
lambdageek
merged commit b15ff06
into
dotnet:mainMay 19, 2022
@ghostghost
locked as resolved and limited conversation to collaborators
Jun 18, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@pavelsavarapavelsavarapavelsavara left review comments
@lateralusXlateralusXlateralusX left review comments
@lewinglewinglewing approved these changes
@thaystgthaystgthaystg approved these changes
@marek-safarmarek-safarAwaiting requested review from marek-safar
@radekdoulikradekdoulikAwaiting requested review from radekdoulik

+1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@lambdageeklambdageek

Labels

arch-wasmWebAssembly architecturearea-System.Diagnostics-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@lambdageek@pavelsavara@lewing@kg@thaystg@lateralusX

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
[wasm-ep] Minimal diagnostic tracing configuration and sample by lambdageek · Pull Request #69158 · dotnet/runtime · GitHub
Skip to content
[wasm-ep] Minimal diagnostic tracing configuration and sample - #69158
Merged
lambdageek merged 38 commits into
dotnet:mainfrom 
lambdageek:wasm-ep
May 19, 2022
Merged
[wasm-ep] Minimal diagnostic tracing configuration and sample#69158
lambdageek merged 38 commits into
dotnet:mainfrom 
lambdageek:wasm-ep
Conversation
@lambdageek
@lambdageeklambdageek
commented
May 10, 2022

Copy link


Copy Markdown

Member
  • Adds a /p:WasmEnablePerfTracing=true configuration.
    In this configuration the runtime is built with threading (MonoWasmThreads property is true), but C# code is not allowed to start threads and doesn't use the portable thradpool (MonoWasmThreadsNoUser property is also true).
    The upshot is that EventPipe can start threads but user code is still single-threaded.
  • Adds a MONO.diagnostics interface in JavaScript. There's a single method for now createEventPipeSession which creates a session that can save a trace to a file on the virtual file system. JS code (or a user in a dev console) needs to call start() and stop() on the session object to begin collecting samples. For now there's no way to specify any configuration options other than the destination VFS file path for the trace file.
  • Adds a sample that runs an async task for five seconds and collects samples and then triggers a click() to download the trace file out of the browser.
@ghostghost
assigned lambdageekMay 10, 2022
@ghost
ghost
commented
May 10, 2022

Copy link


Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing

See info in area-owners.md if you want to be subscribed.
Issue Details
  • Adds a /p:WasmEnablePerfTracing=true configuration.
    In this configuration the runtime is built with threading (MonoWasmThreads property is true), but C# code is not allowed to start threads and doesn't use the portable thradpool (MonoWasmThreadsNoUser property is also true).
    The upshot is that EventPipe can start threads but user code is still single-threaded.
  • Adds a MONO.diagnostics interface in JavaScript. There's a single method for now createEventPipeSession which creates a session that can save a trace to a file on the virtual file system. JS code (or a user in a dev console) needs to call start() and stop() on the session object to begin collecting samples. For now there's no way to specify any configuration options other than the destination VFS file path for the trace file.
  • Adds a sample that runs an async task for five seconds and collects samples and then triggers a click() to download the trace file out of the browser.
Author:lambdageek
Assignees:-
Labels:
arch-wasm, area-System.Diagnostics-mono
Milestone:-
@lambdageek
lambdageek
commented
May 10, 2022

Copy link


Copy Markdown

MemberAuthor
/cc @kg@lateralusX
@lambdageek
lambdageek
commented
May 10, 2022

Copy link


Copy Markdown

MemberAuthor
/cc @pavelsavara
@lambdageek
lambdageek
marked this pull request as ready for review
May 10, 2022 20:50
pavelsavara
pavelsavara
reviewed
May 10, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/types.ts
Outdated

@lambdageek
don't run wasm-eventpipe sample on CI lanes without perftracing
e2e1b66
@lambdageek
more whitespace
ded3aeb
@lambdageek
fix eslint warnings, default rundown to true, allow callback for trac…
46c1a7c
…eFilePath option
@lambdageek
add EventPipeSession.getTraceBlob and EventPipeSession.getTraceDataURI
c29915d
two ways of retreiving the collected traces instead of exposing the
emscripten VFS directly.
Probably the Blob one is enough. Is there any reason to also provide
a data URI?
update the sample to use URL.createObjectURL (session.getTraceBlob())
to create the download link
lateralusX
lateralusX
reviewed
May 11, 2022
View reviewed changes
@lateralusXlateralusX
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EventPipe bits looks good, just a couple of small comments/thoughts.
Comment threadsrc/mono/mono/component/event_pipe-stub.c
Outdated

Comment threadsrc/mono/mono/component/event_pipe-stub.c
Outdated

@azure-pipelines
azure-pipelinesBot
commented
May 16, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@lambdageek
lambdageek
commented
May 17, 2022

Copy link


Copy Markdown

MemberAuthor
Any other feedback?

/cc @lewing
@lambdageek
lambdageek
commented
May 18, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
May 18, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/mono/component/event_pipe.c
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/mono/metadata/threads.c
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/Program.cs
Outdated

kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/index.html
Outdated

kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/main.js
 globalThis.MONO = MONO;
 console.log('after createDotnetRuntime')

 const startWork = BINDING.bind_static_method("[Wasm.Browser.EventPipe.Sample] Sample.Test:StartAsyncWork");
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we rely on the non-Main methods in the sample maybe just put a single line comment in Program.cs's Main to indicate that the reader should look in main.js for the juicy bits
@lambdageeklambdageekMay 18, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/cuint64.ts
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/diagnostics.ts
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/memory.ts
Outdated

}

export function getCU64(offset: _MemOffset): cuint64.CUInt64 {
 const lo = getI32(offset);
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these actually be signed? It seems like they should be unsigned.
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/types/emscripten.ts
}

export declare type TypedArray = Int8Array | Uint8Array | Uint8ClampedArray | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array; No newline at end of file 
export declare type TypedArray = Int8Array | Uint8Array | Uint8ClampedArray | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array;
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also use ArrayBufferView in place of this, though it has the downside of that interface inexplicably not having .length. Just an FYI, I'm fine with us defining a TypedArray alias (do the bigint arrays need to go in here too?)
@lambdageeklambdageekMay 18, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't change anything here. I think eslint just added or removed a newline.
kg
kg
approved these changes
May 18, 2022
View reviewed changes
@lambdageek
lambdageek
commented
May 18, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
May 18, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@lewing
lewing
requested a review
from radekdoulikMay 18, 2022 15:17
lewing
lewing
approved these changes
May 18, 2022
View reviewed changes
@lewinglewing
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:
@lambdageek
lambdageek
merged commit b15ff06
into
dotnet:mainMay 19, 2022
@ghostghost
locked as resolved and limited conversation to collaborators
Jun 18, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@pavelsavarapavelsavarapavelsavara left review comments
@lateralusXlateralusXlateralusX left review comments
@lewinglewinglewing approved these changes
@thaystgthaystgthaystg approved these changes
@marek-safarmarek-safarAwaiting requested review from marek-safar
@radekdoulikradekdoulikAwaiting requested review from radekdoulik

+1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@lambdageeklambdageek

Labels

arch-wasmWebAssembly architecturearea-System.Diagnostics-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@lambdageek@pavelsavara@lewing@kg@thaystg@lateralusX

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
[wasm-ep] Minimal diagnostic tracing configuration and sample by lambdageek · Pull Request #69158 · dotnet/runtime · GitHub
Skip to content
[wasm-ep] Minimal diagnostic tracing configuration and sample - #69158
Merged
lambdageek merged 38 commits into
dotnet:mainfrom 
lambdageek:wasm-ep
May 19, 2022
Merged
[wasm-ep] Minimal diagnostic tracing configuration and sample#69158
lambdageek merged 38 commits into
dotnet:mainfrom 
lambdageek:wasm-ep
Conversation
@lambdageek
@lambdageeklambdageek
commented
May 10, 2022

Copy link


Copy Markdown

Member
  • Adds a /p:WasmEnablePerfTracing=true configuration.
    In this configuration the runtime is built with threading (MonoWasmThreads property is true), but C# code is not allowed to start threads and doesn't use the portable thradpool (MonoWasmThreadsNoUser property is also true).
    The upshot is that EventPipe can start threads but user code is still single-threaded.
  • Adds a MONO.diagnostics interface in JavaScript. There's a single method for now createEventPipeSession which creates a session that can save a trace to a file on the virtual file system. JS code (or a user in a dev console) needs to call start() and stop() on the session object to begin collecting samples. For now there's no way to specify any configuration options other than the destination VFS file path for the trace file.
  • Adds a sample that runs an async task for five seconds and collects samples and then triggers a click() to download the trace file out of the browser.
@ghostghost
assigned lambdageekMay 10, 2022
@ghost
ghost
commented
May 10, 2022

Copy link


Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing

See info in area-owners.md if you want to be subscribed.
Issue Details
  • Adds a /p:WasmEnablePerfTracing=true configuration.
    In this configuration the runtime is built with threading (MonoWasmThreads property is true), but C# code is not allowed to start threads and doesn't use the portable thradpool (MonoWasmThreadsNoUser property is also true).
    The upshot is that EventPipe can start threads but user code is still single-threaded.
  • Adds a MONO.diagnostics interface in JavaScript. There's a single method for now createEventPipeSession which creates a session that can save a trace to a file on the virtual file system. JS code (or a user in a dev console) needs to call start() and stop() on the session object to begin collecting samples. For now there's no way to specify any configuration options other than the destination VFS file path for the trace file.
  • Adds a sample that runs an async task for five seconds and collects samples and then triggers a click() to download the trace file out of the browser.
Author:lambdageek
Assignees:-
Labels:
arch-wasm, area-System.Diagnostics-mono
Milestone:-
@lambdageek
lambdageek
commented
May 10, 2022

Copy link


Copy Markdown

MemberAuthor
/cc @kg@lateralusX
@lambdageek
lambdageek
commented
May 10, 2022

Copy link


Copy Markdown

MemberAuthor
/cc @pavelsavara
@lambdageek
lambdageek
marked this pull request as ready for review
May 10, 2022 20:50
pavelsavara
pavelsavara
reviewed
May 10, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/types.ts
Outdated

@lambdageek
don't run wasm-eventpipe sample on CI lanes without perftracing
e2e1b66
@lambdageek
more whitespace
ded3aeb
@lambdageek
fix eslint warnings, default rundown to true, allow callback for trac…
46c1a7c
…eFilePath option
@lambdageek
add EventPipeSession.getTraceBlob and EventPipeSession.getTraceDataURI
c29915d
two ways of retreiving the collected traces instead of exposing the
emscripten VFS directly.
Probably the Blob one is enough. Is there any reason to also provide
a data URI?
update the sample to use URL.createObjectURL (session.getTraceBlob())
to create the download link
lateralusX
lateralusX
reviewed
May 11, 2022
View reviewed changes
@lateralusXlateralusX
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EventPipe bits looks good, just a couple of small comments/thoughts.
Comment threadsrc/mono/mono/component/event_pipe-stub.c
Outdated

Comment threadsrc/mono/mono/component/event_pipe-stub.c
Outdated

@azure-pipelines
azure-pipelinesBot
commented
May 16, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@lambdageek
lambdageek
commented
May 17, 2022

Copy link


Copy Markdown

MemberAuthor
Any other feedback?

/cc @lewing
@lambdageek
lambdageek
commented
May 18, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
May 18, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/mono/component/event_pipe.c
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/mono/metadata/threads.c
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/Program.cs
Outdated

kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/index.html
Outdated

kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/main.js
 globalThis.MONO = MONO;
 console.log('after createDotnetRuntime')

 const startWork = BINDING.bind_static_method("[Wasm.Browser.EventPipe.Sample] Sample.Test:StartAsyncWork");
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we rely on the non-Main methods in the sample maybe just put a single line comment in Program.cs's Main to indicate that the reader should look in main.js for the juicy bits
@lambdageeklambdageekMay 18, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/cuint64.ts
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/diagnostics.ts
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/memory.ts
Outdated

}

export function getCU64(offset: _MemOffset): cuint64.CUInt64 {
 const lo = getI32(offset);
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these actually be signed? It seems like they should be unsigned.
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/types/emscripten.ts
}

export declare type TypedArray = Int8Array | Uint8Array | Uint8ClampedArray | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array; No newline at end of file 
export declare type TypedArray = Int8Array | Uint8Array | Uint8ClampedArray | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array;
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also use ArrayBufferView in place of this, though it has the downside of that interface inexplicably not having .length. Just an FYI, I'm fine with us defining a TypedArray alias (do the bigint arrays need to go in here too?)
@lambdageeklambdageekMay 18, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't change anything here. I think eslint just added or removed a newline.
kg
kg
approved these changes
May 18, 2022
View reviewed changes
@lambdageek
lambdageek
commented
May 18, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
May 18, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@lewing
lewing
requested a review
from radekdoulikMay 18, 2022 15:17
lewing
lewing
approved these changes
May 18, 2022
View reviewed changes
@lewinglewing
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:
@lambdageek
lambdageek
merged commit b15ff06
into
dotnet:mainMay 19, 2022
@ghostghost
locked as resolved and limited conversation to collaborators
Jun 18, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@pavelsavarapavelsavarapavelsavara left review comments
@lateralusXlateralusXlateralusX left review comments
@lewinglewinglewing approved these changes
@thaystgthaystgthaystg approved these changes
@marek-safarmarek-safarAwaiting requested review from marek-safar
@radekdoulikradekdoulikAwaiting requested review from radekdoulik

+1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@lambdageeklambdageek

Labels

arch-wasmWebAssembly architecturearea-System.Diagnostics-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@lambdageek@pavelsavara@lewing@kg@thaystg@lateralusX

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
[wasm-ep] Minimal diagnostic tracing configuration and sample by lambdageek · Pull Request #69158 · dotnet/runtime · GitHub
Skip to content
[wasm-ep] Minimal diagnostic tracing configuration and sample - #69158
Merged
lambdageek merged 38 commits into
dotnet:mainfrom 
lambdageek:wasm-ep
May 19, 2022
Merged
[wasm-ep] Minimal diagnostic tracing configuration and sample#69158
lambdageek merged 38 commits into
dotnet:mainfrom 
lambdageek:wasm-ep
Conversation
@lambdageek
@lambdageeklambdageek
commented
May 10, 2022

Copy link


Copy Markdown

Member
  • Adds a /p:WasmEnablePerfTracing=true configuration.
    In this configuration the runtime is built with threading (MonoWasmThreads property is true), but C# code is not allowed to start threads and doesn't use the portable thradpool (MonoWasmThreadsNoUser property is also true).
    The upshot is that EventPipe can start threads but user code is still single-threaded.
  • Adds a MONO.diagnostics interface in JavaScript. There's a single method for now createEventPipeSession which creates a session that can save a trace to a file on the virtual file system. JS code (or a user in a dev console) needs to call start() and stop() on the session object to begin collecting samples. For now there's no way to specify any configuration options other than the destination VFS file path for the trace file.
  • Adds a sample that runs an async task for five seconds and collects samples and then triggers a click() to download the trace file out of the browser.
@ghostghost
assigned lambdageekMay 10, 2022
@ghost
ghost
commented
May 10, 2022

Copy link


Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing

See info in area-owners.md if you want to be subscribed.
Issue Details
  • Adds a /p:WasmEnablePerfTracing=true configuration.
    In this configuration the runtime is built with threading (MonoWasmThreads property is true), but C# code is not allowed to start threads and doesn't use the portable thradpool (MonoWasmThreadsNoUser property is also true).
    The upshot is that EventPipe can start threads but user code is still single-threaded.
  • Adds a MONO.diagnostics interface in JavaScript. There's a single method for now createEventPipeSession which creates a session that can save a trace to a file on the virtual file system. JS code (or a user in a dev console) needs to call start() and stop() on the session object to begin collecting samples. For now there's no way to specify any configuration options other than the destination VFS file path for the trace file.
  • Adds a sample that runs an async task for five seconds and collects samples and then triggers a click() to download the trace file out of the browser.
Author:lambdageek
Assignees:-
Labels:
arch-wasm, area-System.Diagnostics-mono
Milestone:-
@lambdageek
lambdageek
commented
May 10, 2022

Copy link


Copy Markdown

MemberAuthor
/cc @kg@lateralusX
@lambdageek
lambdageek
commented
May 10, 2022

Copy link


Copy Markdown

MemberAuthor
/cc @pavelsavara
@lambdageek
lambdageek
marked this pull request as ready for review
May 10, 2022 20:50
pavelsavara
pavelsavara
reviewed
May 10, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/types.ts
Outdated

@lambdageek
don't run wasm-eventpipe sample on CI lanes without perftracing
e2e1b66
@lambdageek
more whitespace
ded3aeb
@lambdageek
fix eslint warnings, default rundown to true, allow callback for trac…
46c1a7c
…eFilePath option
@lambdageek
add EventPipeSession.getTraceBlob and EventPipeSession.getTraceDataURI
c29915d
two ways of retreiving the collected traces instead of exposing the
emscripten VFS directly.
Probably the Blob one is enough. Is there any reason to also provide
a data URI?
update the sample to use URL.createObjectURL (session.getTraceBlob())
to create the download link
lateralusX
lateralusX
reviewed
May 11, 2022
View reviewed changes
@lateralusXlateralusX
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EventPipe bits looks good, just a couple of small comments/thoughts.
Comment threadsrc/mono/mono/component/event_pipe-stub.c
Outdated

Comment threadsrc/mono/mono/component/event_pipe-stub.c
Outdated

@azure-pipelines
azure-pipelinesBot
commented
May 16, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@lambdageek
lambdageek
commented
May 17, 2022

Copy link


Copy Markdown

MemberAuthor
Any other feedback?

/cc @lewing
@lambdageek
lambdageek
commented
May 18, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
May 18, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/mono/component/event_pipe.c
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/mono/metadata/threads.c
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/Program.cs
Outdated

kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/index.html
Outdated

kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/main.js
 globalThis.MONO = MONO;
 console.log('after createDotnetRuntime')

 const startWork = BINDING.bind_static_method("[Wasm.Browser.EventPipe.Sample] Sample.Test:StartAsyncWork");
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we rely on the non-Main methods in the sample maybe just put a single line comment in Program.cs's Main to indicate that the reader should look in main.js for the juicy bits
@lambdageeklambdageekMay 18, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/cuint64.ts
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/diagnostics.ts
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/memory.ts
Outdated

}

export function getCU64(offset: _MemOffset): cuint64.CUInt64 {
 const lo = getI32(offset);
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these actually be signed? It seems like they should be unsigned.
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/types/emscripten.ts
}

export declare type TypedArray = Int8Array | Uint8Array | Uint8ClampedArray | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array; No newline at end of file 
export declare type TypedArray = Int8Array | Uint8Array | Uint8ClampedArray | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array;
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also use ArrayBufferView in place of this, though it has the downside of that interface inexplicably not having .length. Just an FYI, I'm fine with us defining a TypedArray alias (do the bigint arrays need to go in here too?)
@lambdageeklambdageekMay 18, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't change anything here. I think eslint just added or removed a newline.
kg
kg
approved these changes
May 18, 2022
View reviewed changes
@lambdageek
lambdageek
commented
May 18, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
May 18, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@lewing
lewing
requested a review
from radekdoulikMay 18, 2022 15:17
lewing
lewing
approved these changes
May 18, 2022
View reviewed changes
@lewinglewing
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:
@lambdageek
lambdageek
merged commit b15ff06
into
dotnet:mainMay 19, 2022
@ghostghost
locked as resolved and limited conversation to collaborators
Jun 18, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@pavelsavarapavelsavarapavelsavara left review comments
@lateralusXlateralusXlateralusX left review comments
@lewinglewinglewing approved these changes
@thaystgthaystgthaystg approved these changes
@marek-safarmarek-safarAwaiting requested review from marek-safar
@radekdoulikradekdoulikAwaiting requested review from radekdoulik

+1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@lambdageeklambdageek

Labels

arch-wasmWebAssembly architecturearea-System.Diagnostics-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@lambdageek@pavelsavara@lewing@kg@thaystg@lateralusX

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
[wasm-ep] Minimal diagnostic tracing configuration and sample by lambdageek · Pull Request #69158 · dotnet/runtime · GitHub
Skip to content
[wasm-ep] Minimal diagnostic tracing configuration and sample - #69158
Merged
lambdageek merged 38 commits into
dotnet:mainfrom 
lambdageek:wasm-ep
May 19, 2022
Merged
[wasm-ep] Minimal diagnostic tracing configuration and sample#69158
lambdageek merged 38 commits into
dotnet:mainfrom 
lambdageek:wasm-ep
Conversation
@lambdageek
@lambdageeklambdageek
commented
May 10, 2022

Copy link


Copy Markdown

Member
  • Adds a /p:WasmEnablePerfTracing=true configuration.
    In this configuration the runtime is built with threading (MonoWasmThreads property is true), but C# code is not allowed to start threads and doesn't use the portable thradpool (MonoWasmThreadsNoUser property is also true).
    The upshot is that EventPipe can start threads but user code is still single-threaded.
  • Adds a MONO.diagnostics interface in JavaScript. There's a single method for now createEventPipeSession which creates a session that can save a trace to a file on the virtual file system. JS code (or a user in a dev console) needs to call start() and stop() on the session object to begin collecting samples. For now there's no way to specify any configuration options other than the destination VFS file path for the trace file.
  • Adds a sample that runs an async task for five seconds and collects samples and then triggers a click() to download the trace file out of the browser.
@ghostghost
assigned lambdageekMay 10, 2022
@ghost
ghost
commented
May 10, 2022

Copy link


Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing

See info in area-owners.md if you want to be subscribed.
Issue Details
  • Adds a /p:WasmEnablePerfTracing=true configuration.
    In this configuration the runtime is built with threading (MonoWasmThreads property is true), but C# code is not allowed to start threads and doesn't use the portable thradpool (MonoWasmThreadsNoUser property is also true).
    The upshot is that EventPipe can start threads but user code is still single-threaded.
  • Adds a MONO.diagnostics interface in JavaScript. There's a single method for now createEventPipeSession which creates a session that can save a trace to a file on the virtual file system. JS code (or a user in a dev console) needs to call start() and stop() on the session object to begin collecting samples. For now there's no way to specify any configuration options other than the destination VFS file path for the trace file.
  • Adds a sample that runs an async task for five seconds and collects samples and then triggers a click() to download the trace file out of the browser.
Author:lambdageek
Assignees:-
Labels:
arch-wasm, area-System.Diagnostics-mono
Milestone:-
@lambdageek
lambdageek
commented
May 10, 2022

Copy link


Copy Markdown

MemberAuthor
/cc @kg@lateralusX
@lambdageek
lambdageek
commented
May 10, 2022

Copy link


Copy Markdown

MemberAuthor
/cc @pavelsavara
@lambdageek
lambdageek
marked this pull request as ready for review
May 10, 2022 20:50
pavelsavara
pavelsavara
reviewed
May 10, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/types.ts
Outdated

@lambdageek
don't run wasm-eventpipe sample on CI lanes without perftracing
e2e1b66
@lambdageek
more whitespace
ded3aeb
@lambdageek
fix eslint warnings, default rundown to true, allow callback for trac…
46c1a7c
…eFilePath option
@lambdageek
add EventPipeSession.getTraceBlob and EventPipeSession.getTraceDataURI
c29915d
two ways of retreiving the collected traces instead of exposing the
emscripten VFS directly.
Probably the Blob one is enough. Is there any reason to also provide
a data URI?
update the sample to use URL.createObjectURL (session.getTraceBlob())
to create the download link
lateralusX
lateralusX
reviewed
May 11, 2022
View reviewed changes
@lateralusXlateralusX
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EventPipe bits looks good, just a couple of small comments/thoughts.
Comment threadsrc/mono/mono/component/event_pipe-stub.c
Outdated

Comment threadsrc/mono/mono/component/event_pipe-stub.c
Outdated

@azure-pipelines
azure-pipelinesBot
commented
May 16, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@lambdageek
lambdageek
commented
May 17, 2022

Copy link


Copy Markdown

MemberAuthor
Any other feedback?

/cc @lewing
@lambdageek
lambdageek
commented
May 18, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
May 18, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/mono/component/event_pipe.c
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/mono/metadata/threads.c
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/Program.cs
Outdated

kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/index.html
Outdated

kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/main.js
 globalThis.MONO = MONO;
 console.log('after createDotnetRuntime')

 const startWork = BINDING.bind_static_method("[Wasm.Browser.EventPipe.Sample] Sample.Test:StartAsyncWork");
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we rely on the non-Main methods in the sample maybe just put a single line comment in Program.cs's Main to indicate that the reader should look in main.js for the juicy bits
@lambdageeklambdageekMay 18, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/cuint64.ts
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/diagnostics.ts
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/memory.ts
Outdated

}

export function getCU64(offset: _MemOffset): cuint64.CUInt64 {
 const lo = getI32(offset);
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these actually be signed? It seems like they should be unsigned.
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/types/emscripten.ts
}

export declare type TypedArray = Int8Array | Uint8Array | Uint8ClampedArray | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array; No newline at end of file 
export declare type TypedArray = Int8Array | Uint8Array | Uint8ClampedArray | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array;
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also use ArrayBufferView in place of this, though it has the downside of that interface inexplicably not having .length. Just an FYI, I'm fine with us defining a TypedArray alias (do the bigint arrays need to go in here too?)
@lambdageeklambdageekMay 18, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't change anything here. I think eslint just added or removed a newline.
kg
kg
approved these changes
May 18, 2022
View reviewed changes
@lambdageek
lambdageek
commented
May 18, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
May 18, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@lewing
lewing
requested a review
from radekdoulikMay 18, 2022 15:17
lewing
lewing
approved these changes
May 18, 2022
View reviewed changes
@lewinglewing
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:
@lambdageek
lambdageek
merged commit b15ff06
into
dotnet:mainMay 19, 2022
@ghostghost
locked as resolved and limited conversation to collaborators
Jun 18, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@pavelsavarapavelsavarapavelsavara left review comments
@lateralusXlateralusXlateralusX left review comments
@lewinglewinglewing approved these changes
@thaystgthaystgthaystg approved these changes
@marek-safarmarek-safarAwaiting requested review from marek-safar
@radekdoulikradekdoulikAwaiting requested review from radekdoulik

+1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@lambdageeklambdageek

Labels

arch-wasmWebAssembly architecturearea-System.Diagnostics-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@lambdageek@pavelsavara@lewing@kg@thaystg@lateralusX

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Universal Dark Mode - works on any site
(function() {
var enabled = true;
function applyDarkMode() {
if (!enabled) return;
// Create style element if it doesn't exist
var style = document.getElementById('universal-dark-mode-style');
if (!style) {
style = document.createElement('style');
style.id = 'universal-dark-mode-style';
document.head.appendChild(style);
}
// Dark mode CSS - inverts colors but preserves images/video
style.textContent = '
/* Invert everything except media */
html {
filter: invert(1) hue-rotate(180deg) !important;
background: #1a1a2e !important;
}
/* Restore images, videos, iframes, canvas */
img, video, iframe, canvas, svg, picture, [style*="background-image"] {
filter: invert(1) hue-rotate(180deg) !important;
}
/* Preserve specific elements that should not be inverted */
.no-dark-mode, .no-dark-mode *,
[data-theme="light"], [data-theme="light"],
.ace_editor, .ace_editor *,
.CodeMirror, .CodeMirror *,
.monaco-editor, .monaco-editor *,
.markdown-body pre, .markdown-body pre *,
.highlight, .highlight *,
pre code, pre code * {
filter: none !important;
}
/* Fix common UI elements */
.modal, .popup, .dropdown-menu, .tooltip, .popover {
filter: invert(1) hue-rotate(180deg) !important;
background: #2d2d44 !important;
border-color: #444 !important;
}
/* Scrollbars */
::-webkit-scrollbar { background: #1a1a2e !important; }
::-webkit-scrollbar-thumb { background: #444 !important; }
::-webkit-scrollbar-thumb:hover { background: #555 !important; }
/* Selection */
::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
';
}
function removeDarkMode() {
var style = document.getElementById('universal-dark-mode-style');
if (style) style.remove();
}
// Toggle with Alt+Shift+D
document.addEventListener('keydown', function(e) {
if (e.altKey && e.shiftKey && e.key === 'D') {
e.preventDefault();
enabled = !enabled;
if (enabled) {
applyDarkMode();
console.log('[Universal Dark Mode] Enabled');
} else {
removeDarkMode();
console.log('[Universal Dark Mode] Disabled');
}
}
});
// Apply on load
applyDarkMode();
// Re-apply on dynamic content
var observer = new MutationObserver(function(mutations) {
if (enabled && !document.getElementById('universal-dark-mode-style')) {
applyDarkMode();
}
});
observer.observe(document.head, { childList: true });
console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');
})();
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();
[wasm-ep] Minimal diagnostic tracing configuration and sample by lambdageek · Pull Request #69158 · dotnet/runtime · GitHub
Skip to content
[wasm-ep] Minimal diagnostic tracing configuration and sample - #69158
Merged
lambdageek merged 38 commits into
dotnet:mainfrom 
lambdageek:wasm-ep
May 19, 2022
Merged
[wasm-ep] Minimal diagnostic tracing configuration and sample#69158
lambdageek merged 38 commits into
dotnet:mainfrom 
lambdageek:wasm-ep
Conversation
@lambdageek
@lambdageeklambdageek
commented
May 10, 2022

Copy link


Copy Markdown

Member
  • Adds a /p:WasmEnablePerfTracing=true configuration.
    In this configuration the runtime is built with threading (MonoWasmThreads property is true), but C# code is not allowed to start threads and doesn't use the portable thradpool (MonoWasmThreadsNoUser property is also true).
    The upshot is that EventPipe can start threads but user code is still single-threaded.
  • Adds a MONO.diagnostics interface in JavaScript. There's a single method for now createEventPipeSession which creates a session that can save a trace to a file on the virtual file system. JS code (or a user in a dev console) needs to call start() and stop() on the session object to begin collecting samples. For now there's no way to specify any configuration options other than the destination VFS file path for the trace file.
  • Adds a sample that runs an async task for five seconds and collects samples and then triggers a click() to download the trace file out of the browser.
@ghostghost
assigned lambdageekMay 10, 2022
@ghost
ghost
commented
May 10, 2022

Copy link


Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing

See info in area-owners.md if you want to be subscribed.
Issue Details
  • Adds a /p:WasmEnablePerfTracing=true configuration.
    In this configuration the runtime is built with threading (MonoWasmThreads property is true), but C# code is not allowed to start threads and doesn't use the portable thradpool (MonoWasmThreadsNoUser property is also true).
    The upshot is that EventPipe can start threads but user code is still single-threaded.
  • Adds a MONO.diagnostics interface in JavaScript. There's a single method for now createEventPipeSession which creates a session that can save a trace to a file on the virtual file system. JS code (or a user in a dev console) needs to call start() and stop() on the session object to begin collecting samples. For now there's no way to specify any configuration options other than the destination VFS file path for the trace file.
  • Adds a sample that runs an async task for five seconds and collects samples and then triggers a click() to download the trace file out of the browser.
Author:lambdageek
Assignees:-
Labels:
arch-wasm, area-System.Diagnostics-mono
Milestone:-
@lambdageek
lambdageek
commented
May 10, 2022

Copy link


Copy Markdown

MemberAuthor
/cc @kg@lateralusX
@lambdageek
lambdageek
commented
May 10, 2022

Copy link


Copy Markdown

MemberAuthor
/cc @pavelsavara
@lambdageek
lambdageek
marked this pull request as ready for review
May 10, 2022 20:50
pavelsavara
pavelsavara
reviewed
May 10, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/types.ts
Outdated

@lambdageek
don't run wasm-eventpipe sample on CI lanes without perftracing
e2e1b66
@lambdageek
more whitespace
ded3aeb
@lambdageek
fix eslint warnings, default rundown to true, allow callback for trac…
46c1a7c
…eFilePath option
@lambdageek
add EventPipeSession.getTraceBlob and EventPipeSession.getTraceDataURI
c29915d
two ways of retreiving the collected traces instead of exposing the
emscripten VFS directly.
Probably the Blob one is enough. Is there any reason to also provide
a data URI?
update the sample to use URL.createObjectURL (session.getTraceBlob())
to create the download link
lateralusX
lateralusX
reviewed
May 11, 2022
View reviewed changes
@lateralusXlateralusX
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EventPipe bits looks good, just a couple of small comments/thoughts.
Comment threadsrc/mono/mono/component/event_pipe-stub.c
Outdated

Comment threadsrc/mono/mono/component/event_pipe-stub.c
Outdated

@azure-pipelines
azure-pipelinesBot
commented
May 16, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@lambdageek
lambdageek
commented
May 17, 2022

Copy link


Copy Markdown

MemberAuthor
Any other feedback?

/cc @lewing
@lambdageek
lambdageek
commented
May 18, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
May 18, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/mono/component/event_pipe.c
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/mono/metadata/threads.c
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/Program.cs
Outdated

kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/index.html
Outdated

kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/sample/wasm/browser-eventpipe/main.js
 globalThis.MONO = MONO;
 console.log('after createDotnetRuntime')

 const startWork = BINDING.bind_static_method("[Wasm.Browser.EventPipe.Sample] Sample.Test:StartAsyncWork");
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we rely on the non-Main methods in the sample maybe just put a single line comment in Program.cs's Main to indicate that the reader should look in main.js for the juicy bits
@lambdageeklambdageekMay 18, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/cuint64.ts
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/diagnostics.ts
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/memory.ts
Outdated

}

export function getCU64(offset: _MemOffset): cuint64.CUInt64 {
 const lo = getI32(offset);
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these actually be signed? It seems like they should be unsigned.
kg
kg
reviewed
May 18, 2022
View reviewed changes
Comment threadsrc/mono/wasm/runtime/types/emscripten.ts
}

export declare type TypedArray = Int8Array | Uint8Array | Uint8ClampedArray | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array; No newline at end of file 
export declare type TypedArray = Int8Array | Uint8Array | Uint8ClampedArray | Int16Array | Uint16Array | Int32Array | Uint32Array | Float32Array | Float64Array;
@kgkgMay 18, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also use ArrayBufferView in place of this, though it has the downside of that interface inexplicably not having .length. Just an FYI, I'm fine with us defining a TypedArray alias (do the bigint arrays need to go in here too?)
@lambdageeklambdageekMay 18, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't change anything here. I think eslint just added or removed a newline.
kg
kg
approved these changes
May 18, 2022
View reviewed changes
@lambdageek
lambdageek
commented
May 18, 2022

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
May 18, 2022

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@lewing
lewing
requested a review
from radekdoulikMay 18, 2022 15:17
lewing
lewing
approved these changes
May 18, 2022
View reviewed changes
@lewinglewing
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:
@lambdageek
lambdageek
merged commit b15ff06
into
dotnet:mainMay 19, 2022
@ghostghost
locked as resolved and limited conversation to collaborators
Jun 18, 2022
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@pavelsavarapavelsavarapavelsavara left review comments
@lateralusXlateralusXlateralusX left review comments
@lewinglewinglewing approved these changes
@thaystgthaystgthaystg approved these changes
@marek-safarmarek-safarAwaiting requested review from marek-safar
@radekdoulikradekdoulikAwaiting requested review from radekdoulik

+1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@lambdageeklambdageek

Labels

arch-wasmWebAssembly architecturearea-System.Diagnostics-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@lambdageek@pavelsavara@lewing@kg@thaystg@lateralusX