Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wasm] Convert mono-config.js to mono-config.json #53248

Closed
wants to merge 118 commits into from
Closed

[wasm] Convert mono-config.js to mono-config.json #53248

wants to merge 118 commits into from

Conversation

Daniel-Genkin
Copy link
Contributor

@Daniel-Genkin Daniel-Genkin commented May 25, 2021

WORK IN PROGRESS

This PR renames the mono-config.js file to mono-config.json, updates the contents accordingly and moves it's loading from the DOM to JS.

Why is this needed?
This change is step 0 before adding NodeJS support to the wasm runtime. As NodeJS doesn't have a DOM, we need to load the config file from JS. Moreover, by moving the variable declaration out of the file, we can reduce ambiguity as to what the config variable is. This will also have the added benefit of being able to rename the variable if needed.

What's changed

  • mono-config.js has been renamed to mono-config.json and the config variable declaration has been removed.
  • there is now a new js_support.js file that is copied directly (via emssdk pre-js flag) to dotnet.js. This file loads the config file and then fires a callback (onConfigLoaded(config)) once it is loaded with the contents of the config file in the config parameter.
  • the wasm and debugger samples have been updated to work with this change.
  • This PR builds on a bunch of fixes that @lewing made in one of his draft PRs. (Should this be removed as it isn't directly related to this PR?)

How it works
Since I use the --pre-js flag, the load_config() is the first function to execute in dotnet.js (even before the emsdk runtime is initialized). Then the emsdk runtime initializes as normal. Lastly the mono runtime is initialized (load config -> runtime initialized -> load mono runtime with args). This solves the previous open question and a few of the comments. Plus it involves minimal changes to other projects that use wasm as they simply just need to add the snippet below to their existing code to make it work.

    config: null,
    onConfigLoaded: function (config) {
        if (!config || config.error){
            console.log("An error occured while loading the config file");
            return;
        }
        Module.config = config;
    },

Things to consider
Since the config file is loaded asynchronously (or at least may be loaded asynchronously), code that requires the use of the config variable must be run after the callback. Therefore, it cannot simply be placed in script tags within HTML as it could before.
Open questions
Is there a better way to do this loading such that there is no need for the callback? Could we include it in the loading process of wasm? If so, how should that be done?

This has been resolved by loading the config before the runtime init.

Also, note that I am very new to this project (I am an intern) so please let me know if there is anything else that should be improved including coding style, better techniques for loading the files and/or anything else.

@ghost
Copy link

ghost commented May 25, 2021

Tagging subscribers to this area: @CoffeeFlux
See info in area-owners.md if you want to be subscribed.

Issue Details

This PR renames the mono-config.js file to mono-config.json, updates the contents accordingly and moves it's loading from the DOM to JS.

Why is this needed?
This change is step 0 before adding NodeJS support to the wasm runtime. As NodeJS doesn't have a DOM, we need to load the config file from JS. Moreover, by moving the variable declaration out of the file, we can reduce ambiguity as to what the config variable is. This will also have the added benefit of being able to rename the variable if needed.

What's changed

  • mono-config.js has been renamed to mono-config.json and the config variable declaration has been removed.
  • there is now a new JSSupportLibrary module that is copied directly (via emssdk post-js flag) to dotnet.js. This module has a function load_config which loads the config file and then fires a callback once it is loaded with the contents of the config file.
  • the wasm and debugger samples have been updated to work with this change.
  • This PR builds on a bunch of fixes that @lewing made in one of his draft PRs.

Things to consider
Since the config file is loaded asynchronously (or at least may be loaded asynchronously), code that requires the use of the config variable must be run after the callback. Therefore, it cannot simply be placed in script tags within HTML as it could before.

Open questions

  • Is there a better way to do this loading such that there is no need for the callback? Could we include it in the loading process of wasm? If so, how should that be done?

Also, note that I am very new to this project (I am an intern) so please let me know if there is anything else that should be improved including coding style, better techniques for loading the files and/or anything else.

Author: Daniel-Genkin
Assignees: -
Labels:

area-VM-meta-mono

Milestone: -

@dnfadmin
Copy link

dnfadmin commented May 25, 2021

CLA assistant check
All CLA requirements met.

@thaystg thaystg added the arch-wasm WebAssembly architecture label May 25, 2021
@ghost
Copy link

ghost commented May 25, 2021

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

Issue Details

WORK IN PROGRESS

This PR renames the mono-config.js file to mono-config.json, updates the contents accordingly and moves it's loading from the DOM to JS.

Why is this needed?
This change is step 0 before adding NodeJS support to the wasm runtime. As NodeJS doesn't have a DOM, we need to load the config file from JS. Moreover, by moving the variable declaration out of the file, we can reduce ambiguity as to what the config variable is. This will also have the added benefit of being able to rename the variable if needed.

What's changed

  • mono-config.js has been renamed to mono-config.json and the config variable declaration has been removed.
  • there is now a new JSSupportLibrary module that is copied directly (via emssdk post-js flag) to dotnet.js. This module has a function load_config which loads the config file and then fires a callback once it is loaded with the contents of the config file.
  • the wasm and debugger samples have been updated to work with this change.
  • This PR builds on a bunch of fixes that @lewing made in one of his draft PRs.

Things to consider
Since the config file is loaded asynchronously (or at least may be loaded asynchronously), code that requires the use of the config variable must be run after the callback. Therefore, it cannot simply be placed in script tags within HTML as it could before.

Open questions

  • Is there a better way to do this loading such that there is no need for the callback? Could we include it in the loading process of wasm? If so, how should that be done?

Also, note that I am very new to this project (I am an intern) so please let me know if there is anything else that should be improved including coding style, better techniques for loading the files and/or anything else.

Author: Daniel-Genkin
Assignees: -
Labels:

arch-wasm, area-VM-meta-mono

Milestone: -

src/mono/wasm/wasm.proj Outdated Show resolved Hide resolved
src/mono/wasm/runtime/js_support.js Outdated Show resolved Hide resolved
src/mono/wasm/runtime/js_support.js Outdated Show resolved Hide resolved
Comment on lines 297 to 301
if (is_node) {
eval (read ("dotnet.js").toString());
} else {
loadScript ("dotnet.js");
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we fold this into loadScript itself?

Daniel-Genkin and others added 4 commits June 1, 2021 13:51
* Moved R2RDump statistics from console to --out file

* Automated the fixup statistics report into a self-maintaining component.

* Optimized the minimum values by setting them at the beginning, and removing the Math.Max() calls.
jkotas and others added 26 commits June 1, 2021 13:56
* Hot Reload: test on WebAssembly

   1. Always build the assemblies in the ApplyUpdate/ subdirectory without optimization, with debug info.  Hot reload depends on it.
   2. Pass the required environment variable to the runtime via xharness to enable support for applying updates.

* Add ApplyUpdate test assemblies to test project linker descriptor

* Fix wasm EnableAggressiveTrimming lane
* Parse DOTNET_PROCESSOR_COUNT with a 10 radix not 16

* Update test for DOTNET_PROCESSOR_COUNT
…vents on Mono. (#53020)

* Add native JIT event into EventPipe.

* Emit module/assembly load native events into EventPipe.

* Emit MethodDCEnd_V1 and MethodILToNativeMap EventPipe events.
#3601
Bundling should generate id based on the content in order to secure unique but reproducible ids
We're still having capacity issues even after doubling the number of devices.  Only run device tests on the rolling build for the time being.

Co-authored-by: Steve Pfister <steve.pfister@microsoft.com>
- More code sharing for System.Exception
- Some support for default interface methods
- Move callling convention helpers around to avoid duplication
- Unify Environment.StackTrace
…eue (#52771)

* Clean up the ExternalObjectContext cache for ComWrappers' RCWs.

* Add testing for verifying internal cache clean up.
SymReader::InitializeFromFile opens file handles which were not closed causing resource leaks on each call.

Fix #50422

Co-authored-by: Artem Kliatchkine <eldog@rambler.ru>
create SSL dev cert via powershell on helix
* Reduce DefaultPooledConnectionIdleTimeout default

- Change DefaultPooledConnectionIdleTimeout from 120 seconds to 60 seconds
- This should reduce the occurrence of socket errors when connected to IIS or Kestrel
@Daniel-Genkin
Copy link
Contributor Author

Daniel-Genkin commented Jun 1, 2021

Just to clarify what just happened, I originally made this branch from one of Larry's. So now I rebased to remove Larry's changes and then merged the current dotnet/runtime main into this branch.

EDIT: This inadvertently made the PR a mess. So I am going to close this one and open another that will do the same thing but be much more clear with it's history and number of commits.

@Daniel-Genkin
Copy link
Contributor Author

Closing as I have made a new PR that is cleaner: #53606

@ghost ghost locked as resolved and limited conversation to collaborators Jul 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-VM-meta-mono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet