[SPARK-11249][launcher] Throw error if app resource is not provided.

Without this, the code would build an invalid spark-submit command line,
and a more cryptic error would be presented to the user. Also, expose
a constant that allows users to set a dummy resource in cases where
they don't need an actual resource file; for backwards compatibility,
that uses the same "spark-internal" resource that Spark itself uses.
Tested via unit tests, run-example, spark-shell, and running the
thrift server with mixed spark and hive command line arguments.

Test build #57807 has started for PR 12909 at commit 0ab4a28.

Test build #57807 has finished for PR 12909 at commit 0ab4a28.

This patch fails from timeout after a configured wait of `250m`.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57807/
Test FAILed.

retest this please

Test build #57900 has started for PR 12909 at commit 0ab4a28.

Test build #57900 has finished for PR 12909 at commit 0ab4a28.

This patch fails from timeout after a configured wait of `250m`.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57900/
Test FAILed.

Hmm, pyspark seems to be getting stuck, let me see if I broke it...

Test build #58030 has started for PR 12909 at commit 46664ba.

Test build #58030 has finished for PR 12909 at commit 46664ba.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58030/
Test FAILed.

retest this please

Test build #58039 has started for PR 12909 at commit 46664ba.

Test build #58039 has finished for PR 12909 at commit 46664ba.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58039/
Test PASSed.

@andrewor14

Merging to master / 2.0.


Support for IL interpreter in CoreRT

We should consider porting Mono's .NET interpreter (http://www.mono-project.com/news/2017/11/13/mono-interpreter/) over to CoreRT, to support runtime IL generation and execution within native code

The interpreter implementation is always tighly coupled with the underlying runtime implemenentation details. It cannot be easily ported from runtime to runtime. This would be more like a rewrite, maybe using some of the ideas from the other implementations. We would want to rewrite it in C# for CoreRT. Implementing it in C/C++ for CoreRT would be much harder than implementing it in C#.
BTW: We have interpreter in CoreCLR as well: https://github.com/dotnet/coreclr/blob/master/src/vm/interpreter.cpp . It is not shipping in .NET Core because of .NET Core would not benefit from it. It has been used for new platform bringups, and various experiments.
There are two distinct pieces:

IL interpreter: The interpreter should not be tightly coupled with Reflection.Emit. It should be able to interpret any IL, e.g. even IL comming from regular IL .dlls. The idea is that we would have pluggable execution strategy for the IL. Some of the early plumbing for this has been done in our System.Private.Jit prototype - look for MethodExecutionStrategy. The interpreter can live in say System.Private.Interpreter and be pluged in as execution strategy. A good first step would be to put all scafolding in place that is able to interpret a super simple method that just returns "42" end-to-end. Let's use this issue to track further dicusssion on this.

Refleflection.Emit: Reflection.Emit should be independent part as described in: https://github.com/dotnet/corefx/issues/4491#issuecomment-189756092 . Let's use this corefx issue for further discussion on Reflection.Emit. The "Run" flavor of Reflection.Emit should communicate with the runtime by passing PE files down. In case of DynamicMethods, it can pass down just baked IL method bodies as performance optimization since producing a PE file per dynamic method would be too expensive.

@jkotas looking to take a first pass at this. However, I have a couple of questions:

The OnEntryPoint method returns an IntPtr which I'm guessing points to the native compiled code. How will this work in the context of the interpreter.
Considering CoreRT ships without a JIT, at what point is the RyuJitExecutionStrategy used? Where in the code can I find it?

The OnEntryPoint method returns an IntPtr

CoreRT has capability to manufacture entrypoints. Look for CallConverterThunk.MakeThunk for an example. The call converter is sort of a mini-interpreter that just changes once calling convention to a different one.

RyuJitExecutionStrategy

Code under src\System.Private.Jit was a prototype for JIT support in CoreRT. It was able to run a few methods, nothing extensive. It does not ship, and it is not used in the default CoreRT config and it has likely bit-rotten and does not work anymore.

Sweet! thanks Matt! Will take a look.

Yup, that definitely answers my question, know there had to be an FCALL somewhere

I've looked into the equivalent flow for CoreRT (mostly just for my own interest) and unless I'm missing something it's all in C#, which is pretty cool!

MethodInvoker - Object Invoke(..)
MethodInvoker - protected abstract Object Invoke(..)

StaticMethodInvoker - override Object Invoke(..)
InstanceMethodInvoker - override Object Invoke(..)
VirtualMethodInvoker - override Object Invoke(..)

RuntimeAugments - CallDynamicInvokeMethod(..)
InvokeUtils - CallDynamicInvokeMethod(..)
CalliIntrinsics - Call(..)

At this point there are no C# method bodies, but after a bit of digging, it seems that the [McgIntrinsics] attribute is a placeholder and the method bodies are wired up via ILProvider -  CreateMethodIL(MethodDesc method) and finally CalliIntrinsic - EmitIL(MethodDesc target) (using OpCodes.Calli)

You should write a blog post on this 😃

It looks like you're on the right track with the implementation details in System.Private.Reflection.Execution (no FCALLs in CoreRT). @MichalStrehovsky or @davidwrighton, do you have any details to fill in?

Just out of interest, from reading this post there seems to be 2 possible approaches to allowing runtime code generation in CoreRT:

Implement an IL Interpreter
Use RyuJIT (i.e. System.Private.Jit)

I was wondering what's the preferred approach? Or in an ideal world would you have both? (leaving aside the cost to implement, engineering time, other priorities, etc)
Is 2) less preferable because it's no longer 'AOT compiled' or is that not really a concern? I assume that 1)  is slower, but does that matter?

Interpreter is good to have for places where (Ryu)JIT is not available, not allowed (e.g. Apple devices), or not desirable (e.g. environments locked down for security).
It may be also used as part of tiered code generation strategy.

@jkotas Thanks for the info
@tonerdo BTW you may find these 2 blog posts help you out and/or give you something to think about when working on this issue:

The .NET IL Interpreter
DotNetAnywhere: An Alternative .NET Runtime

So how does the runtime figure out the connection? How do SetThunkData and related methods actually work?

SetThunkData sets what the low-level code should call back when the thunk is actually called.
I have found that we have this actually wrapped in more easy to use type: abstract class CallInterceptor.

The only thing is I'm not sure exactly how to use it. Does it point to the starting memory address the arguments are stored in?

Yes, it points to the memory block where arguments are stored in.
I have found that we have this actually wrapped in more easy to use type: abstract class CallInterceptor. Can you try the following?

Inherit from CallInterceptor and implement the abstract methods. For a quick experiment, just make them return some fixed arrays.
Create instance of the CallInterceptor and call GetThunkAddress() on it. It will give you the address of the method.
Call this method using ...
You should see your overiden ThunkExecute method getting called. And the CallInterceptorArgs should give you access to the arguments.

Thanks for info, been a bit busy with work. Will take a first pass over the weekend

How do I get the information (e.g. MethodDesc) of the method being intercepted?

Store it as a field in your type inherited from CallInterceptor.

How will the IL of the method be passed to the CallInterceptor?

It should be computed from the MethodDesc. I would just make something simple to make the prototype work.
Eventually, it may need to handle IL coming from all sort of different places - something like https://github.com/dotnet/corert/blob/fb17c5fa11f780e72090f2c987c82c541a0741b0/src/Common/src/TypeSystem/IL/ILProvider.cs#L239 .

@tonerdo Yes, you got it. I do not see anything wrong in your description.

Hi @jkotas, I've been able to put all the scaffolding in place to interpret a simple method that returns an integer/float/double constant.
E.g.
int SomeMethod()
{
    return 20;
}

What's the best way to test out this functionality?

That sounds like a good start, @tonerdo. A good way to start might be adding a test project that loads a dll and interprets a test method out of it. That way, as more opcodes come in, it's easy to add more tests.

From a quick look at your changes, it generally looks like you're on the right track, but you might want to consider looking at the ILImporter from ILCompiler. It handles finding basic blocks and exception regions as well as instruction decoding and some stack management, so it will help when you get to flow control and potentially simplify some of the other work as well.

Thanks for the answer and pointers @morganbr, will definitely take a look. I'm definitely on board with the test project, my question now is how I'm going to integrate the test project with the ReflectionExecution pipeline so that things like method entrypoint thunks are setup correctly

@jkotas thanks for the pointer but I'm afraid I'm still not entirely clear. I have my GlobalExecutionStrategy set to the interpreter and then I add it to AutoInitializedAssemblies but I'm at a loss on how exactly to use it.
Let me be a bit clearer, assuming I have a simple native console app that loads a .NET dll and extracts a simple method as a byte array (as @morganbr suggested) what I'm at a loss of is what runtime method to call to get CoreRT to setup and invoke my execution strategy.
How do I pass in the IL bytes of the method as well as the method information to CoreRT so that it sets up the EntryPoint thunks etc? Do I have to publicly expose certain parts of the runtime (just for preliminary testing purposes)? Do I need to setup intrinsics like is done for Method.Invoke(...)? How was System.Private.Jit tested?

I think you can just use Assembly.Load and other reflection to call it with roughly the work you've already done. You probably need to set your execution strategy here though (outside of the disabled JIT ifdef):
https://github.com/dotnet/corert/blob/e19bace28a18a0d39a451bb0f0dbd325551508e1/src/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/ReflectionExecution.cs#L69

How was System.Private.Jit tested?

It was tested using Assembly.Load(byte[]). This method is wired to call back into System.Private.Reflection.Execution.

Awesome! That's exactly what I've been looking for. Thanks guys

The Assembly.Load(byte[]) path might have gotten a bit of bit rot since it was last exercised. When I tried to stand up the JIT prototype again a couple weeks ago, I hit an issue where Load was failing because we ended up registering the ECMA module twice (with the first registration being incomplete). I didn't actually get to debugging it because my wife had different plans for me at that time, but it didn't look too scary and was pretty easy to locate when stepping through the Load call. You might hit the same thing.

Ah thanks for pointing that out @MichalStrehovsky. Just a quick question though, are the *.Experimental.dll versions of the libraries used by default? I have everything in System.Private.Reflection.Execution.Experimental.dll all stood up to use the interpreter , just need to know if I have to do anything to have that be the default instead of System.Private.Reflection.Execution.dll

The composition of the class libraries is specified here:
https://github.com/dotnet/corert/blob/57289ff33dba98c8aea472019adebc08ec68634b/src/BuildIntegration/Microsoft.NETCore.Native.targets#L82-L87
The list specifies what assemblies need to run initialization as part of startup. This is where the execution strategy will get hooked up.
I would make the inclusion of System.Private.Reflection.Execution and System.Private.TypeLoader conditional on something like '$(ExperimentalInterpreterSupport)' != 'true' and swap in the Experimental version of those two if the property is true. We do not want to be compiling the non-experimental versions in that case.

I'm seeing a very weird error on macOS:
/Users/toni/Workspace/corert/Tools/sign.targets(113,5): error : /Users/toni/Workspace/corert/bin/obj/OSX.x64.Debug/ILCompiler.Build.Tasks/ILCompiler.Build.Tasks.dll: PE file is already strong-name signed. [/Users/toni/Workspace/corert/src/ILCompiler.Build.Tasks/src/ILCompiler.Build.Tasks.csproj]

What makes it weird is that I get this error even though I haven't made any changes to any corert build scripts

Try deleting bin directory and doing a clean build.

Could this be the bit rot you were referring to @MichalStrehovsky?

Yes, that's the one. GetRuntimeAssemblyFromByteArray should not be throwing. If you step through it, it should eventually land here, if I remember correctly:
https://github.com/dotnet/corert/blob/2b40a707239a9974a63cf992ce6a6f8303f5b362/src/System.Private.TypeLoader/src/Internal/Reflection/Execution/AssemblyBinderImplementation.Ecma.cs#L62-L82
(Or some other code that was doing a very similar thing - I'm not 100% sure.)
The problem seemed to be that the first call to Bind correctly didn't find the assembly. We then inserted it into the list, but the second call to Bind (// 5.

Thanks for pointing out the problem @MichalStrehovsky, what's the best way to fix this?

Thanks for pointing out the problem @MichalStrehovsky, what's the best way to fix this?

I would start with finding out why there's an incomplete entry in the list. I didn't debug this any further than that.

@MichalStrehovsky after a bit of debugging I believe I've figured out where the problem is:
At the point where the PEInfo object of the loaded assembly is inserted into s_ecmaLoadedAssemblies the Reader property attached to the object is not null.
https://github.com/tonerdo/corert/blob/52570eba361616c122b2d1eb7a244402f98cb615/src/System.Private.TypeLoader/src/Internal/Reflection/Execution/AssemblyBinderImplementation.Ecma.cs#L69-L75
However, when that object is later retrieved after a match is found the Reader property attached to the returned PEInfo object is null and this caused the method to return false even though a match was found
https://github.com/tonerdo/corert/blob/52570eba361616c122b2d1eb7a244402f98cb615/src/System.Private.TypeLoader/src/Internal/Reflection/Execution/AssemblyBinderImplementation.Ecma.cs#L104-L105
I'm a bit stumped as to why this field becomes null because further investigation didn't yield any points in the code where the original object reference (and other references) for that field was made null

At the point where the PEInfo object of the loaded assembly is inserted into s_ecmaLoadedAssemblies the Reader property attached to the object is not null.

Just a random thought - did you check the that the object identity is the same? I.e. we didn't accidentally create two PEInfo somewhere and put the wrong one in the cache (or put both in the cache).

@MichalStrehovsky walked through it again and that possibility doesn't jump out at me

Does the first PEInfo object have same address as the second PEInfo object? You can try setting a write breakpoint to tell where the returned PEInfo object becomes null.

@jkotas That sounds like a good approach. Can you please give me some pointers how to do that from WinDbg?

ba w8 <address>

Further investigation shows that the second PEInfo object has a different address from the first. A write breakpoint on the first PEInfo object is never hit.

@MichalStrehovsky I've looked deeper into the issue and here's what I've found
At the first attempt to bind to the already loaded assembly
https://github.com/dotnet/corert/blob/master/src/System.Private.TypeLoader/src/Internal/Reflection/Execution/AssemblyBinderImplementation.Ecma.cs#L61-L62
s_ecmaLoadedAssemblies already contains a single PEInfo object with origins I'm not quite sure of.
The call to s_ecmaLoadedAssemblies.Add(peInfo) doesn't actually add the peInfo object, which leads me to believe that there's a low level bug in the Add method of LowLevelList. What this means is that the PEInfo object that somehow appeared in the list is always returned because it happens to be a match, but the method will always fail because the returned object has relevant fields set to null
Re-initializing s_ecmaLoadedAssemblies just before the call to s_ecmaLoadedAssemblies.Add(peInfo) fixes the issue and Assembly.Load works just fine. This is just a band-aid fix of course, for a more permanent I'll need to find answers to the following questions

Where does the initial PEInfo item in  s_ecmaLoadedAssemblies come from?
Why does the Add method not add when the above item is in the list? (I might be able to figure this out from deeper debugging)

Anyways, the following code:
byte[] buffer = File.ReadAllBytes(path);
var assembly = Assembly.Load(buffer);

now works fine with the Experimental assemblies swapped in, can move on with the interpreter testing
cc @jkotas

I put this under debugger (setting a memory breakpoint on the count field of LowLevelList) and found the place that adds the dummy:
https://github.com/dotnet/corert/blob/2b40a707239a9974a63cf992ce6a6f8303f5b362/src/System.Private.TypeLoader/src/Internal/Reflection/Execution/AssemblyBinderImplementation.Ecma.cs#L188-L193
Seems like this is code that was added when support for satellite assemblies was added.
Commenting that piece out also fixes it.
I went ahead and tried to resurrect the JIT prototype (my notes might be useful in your effort as well):

Comment out the piece above
ILC needs to run with --nometadatablocking option because we can't do reflection metadata blocking when interpreting or jitting. (You'll see issues finding Object.Finalize during type loading at runtime if you don't do that)
I had to copy over jitinterface.dll and clrjitilc.dll next to the EXE
Found an issue with factoring of System.Private assemblies. System.Private.Interop that we include is still compiled against System.Private.TypeLoader (found out the hard way, because we were crashing due to a null TypeLoaderEnvironment - this is because this other copy of the type loader was not initialized). We need to make a System.Private.Interop.Experimental assembly and include that. Turns out there's a bunch more things that need .Experimental flavors. I stopped my attempts there for now.

I've added a System.Private.Interop.Experimental project file with similar format to the other Experimental projects. However, for some weird reason that I can't seem to figure out, no System.Private.Interop.Experimental.dll assembly is generated.
Is there some special way this project is included into the build system?

There isn't anything magic that I know of. Maybe add a reference to it from the System.Private.Jit project? Not sure if that would help, but the project is currently referencing System.Private.Interop, but should be referencing the .Experimental flavor.

@MichalStrehovsky what's the best way to make an assembly part of _scopeGroups?
https://github.com/dotnet/corert/blob/master/src/System.Private.TypeLoader/src/Internal/Reflection/Execution/AssemblyBinderImplementation.cs#L76-L79
I'm running into assembly binding failures because System.Runtime isn't present in that list. My first thought was to use AutoInitializedAssemblies  but that won't work because System.Runtime.dll isn't present in the sdk build folder.
I get the exception thrown when trying to get the EntryPoint from the successfully loaded assembly
byte[] buffer = File.ReadAllBytes(@"C:\Users\toni\Workspace\cscore\bin\Debug\netcoreapp2.0\CsCore.dll");
var assembly = Assembly.Load(buffer);
var entryPoint = assembly.EntryPoint;

@MichalStrehovsky what's the best way to make an assembly part of _scopeGroups?
https://github.com/dotnet/corert/blob/master/src/System.Private.TypeLoader/src/Internal/Reflection/Execution/AssemblyBinderImplementation.cs#L76-L79
I'm running into assembly binding failures because System.Runtime isn't present in that list. My first thought was to use AutoInitializedAssemblies  but that won't work because System.Runtime.dll isn't present in the sdk build folder.
I get the exception thrown when trying to get the EntryPoint from the successfully loaded assembly
byte[] buffer = File.ReadAllBytes(@"C:\Users\toni\Workspace\cscore\bin\Debug\netcoreapp2.0\CsCore.dll");
var assembly = Assembly.Load(buffer);
var entryPoint = assembly.EntryPoint;

cc @jkotas

Use RD.XML to force it into the metadata (RD.XML that the CoreRT compiler understands looks like this, you add it to the project like this and the RD.XML that should help for this is <Assembly Name="System.Runtime" /> - you shouldn't need any directives under it).
If it still doesn't work, check if Type.GetType("System.Object, System.Runtime") returns non-null with this set up for a plain, non-interpreter project.

@MichalStrehovsky that did the trick! It works now and my interpreter code is hit when I call Invoke on the entry point, it shows I still have to fix up some things. Won't it be a bit more convenient to have System.Runtime included by default, considering it contains foundational runtime types.

@jkotas @MichalStrehovsky I've been able to get the interpreter fully integrated and working quite well. It's currently very limited and only supports ldc.i* and ret opcodes. I've opened a PR at #6182 so you can validate code structure as well as approach so I can move on to more complex scenarios.
Would you prefer I open separate issues for adding support for additional opcodes (similar to what @morganbr does for WASM support) or would you prefer this remains the only tracking issue.

I think separate issues is a good idea if you:

Want help from other folks to jump in and implement one or two opcodes (which is part of why I did that for WASM)
Plan to get through lots of them so they don't hang around forever
Put in details so that somebody can pick them up (feel free to copy mine and adapt as necessary)

I also skipped opcodes that we're not likely to need for a very long time or ever (like argiterator) to avoid clutter.


Dependencies Updated

Updated dependencies,
Font Awesome was updated to Version 5
babel/core was removed since you are using @babel/core

@JamesVanWaza Looks like there are some conflicts. Can you please rebase and resolve this issue.

@cameronadams777 Done

@JamesVanWaza I see that you have pushed a few new commits here. Are you just periodically updating until this is merged or are you still actively working on this?

Periodically updating

@JamesVanWaza When trying this out to make sure that nothing was broken before approving and merging, I am receiving this error:

Are you seeing this as well? And if so, are you checking that the application still builds after updating these modules?

@cameronadams777 yes I got the same error. Still trying to figure that out.

@cameronadams777, any luck, with the version error?

@JamesVanWaza yeah I was able to fix that. There are several issues I'm still working out. The current issue being with the update to Monaco. If you would like, I can push up my changes and let you continue working on it.

That would be good

@cameronadams777 still waiting for pull request

@JamesVanWaza I have updated a few more dependencies, added some dependencies that are needed that since this project was open sources, have been labeled as peer dependencies but there are still some snags I was running into with the css-loaders. That said, I am pretty limited on time these days (I apologize for taking this long to even get this commit in) so can you please take a look?

@cameronadams777 Will check did you send the updated pull request?

Are you getting this error as well?

@JamesVanWaza I went ahead and made the change to your fork as it seems that PRs are not available for the repo. That said I am not seeing that error. Maybe try deleting node_modules and yarn.lock and reinstalling.


Update markdown.jsx for numbers in uris (master)

Same as #135 but rebased onto master.
closes #16

I don't think any action is required here.  The 4.3 branch will get merged into master soon.