-
Notifications
You must be signed in to change notification settings - Fork 123
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
corlib and JIT updates #7
base: master
Are you sure you want to change the base?
Conversation
This looks like a good pull request. I'm excited for this project |
C# client side ?? Seriously this looks great. |
That's awesome! By the way, do we have Blazor on official Core 2.0 instead of preview version? |
+1 adding my support for this project. Reaaaally want to see it come to fruition! |
How can one help with this project? Because I seriously want to see this getting bigger :) |
+1 @Huarru I too would love to know how I can help. I really think this can be a solution for the messed up, mangled mess that we today call web development. https://twitter.com/thomasfuchs/status/708675139253174273?lang=el |
I tried implementing a new feature but the missing reflection features are blocking me. I tried using DNA from this PR but still there are a lot of basic reflection stuff missing. I think the first thing to do should be extending DNA, although I am not sure if this is the way to go since Mono is also working to bring their runtime to wasm. http://www.mono-project.com/news/2017/08/09/hello-webassembly/ Maybe switch to Mono runtime ? (when ready) |
Here's why reflection in this type of project is not desirable:. Reflection prevents a linker from pruning code unless done very carefully. It is imparitive that blazor implements a linker to shrink the size and using DNA keeps the monster size of mono out of this. (see how large the same app is for xamarin Android versus Java if you have any questions and that's with a linker) On the web all you need is event handling, http requesting, and operations in the Dom (and cookies, storage I etc) all with strong typing. Essentially everything else is noise that should be done server side. If a model that uses razor syntax for binding with eventing can be created that matches angular but does so with dnx and web assembly using mvc pages and mvc routing we'd have a massive win and would forever change web development, which is a disaster right now) for the better. And as an aside, Al of this has the benefit of being natively executable for server side pre-rendered for SEO with seamless handoff and no nodejs involved which is another HUGE win. |
@JohnGalt1717 Ok, that makes sense |
+1 for this project. I'm waiting with baited breath. |
I get that reflection is tricky when it comes to dead code elimination, but it's also an integral part of many codebases. Perhaps an approach like .NET Native's "Runtime Directives" can be used? https://docs.microsoft.com/en-us/dotnet/framework/net-native/reflection-and-net-native |
PS if anyone needs an extra hand for this, sign me up. |
What kind of knowledge is required to contribute to this project? Does anyone have any recommendations on which books I should read? I think rather than wasting countless hours writing awkward JavaScript/TypeScript code, I would rather spend those hours contribute to this project. |
@SteveSanderson Separated the Blazor-Hackaton baseline to help the review. |
"I think rather than wasting countless hours writing awkward JavaScript/TypeScript code, I would rather spend those hours contribute to this project.”
Amen to that.
|
Just saw the announcement of an official .NET IL Linker at dotnet/core#915. That seems like great news for this project to reduce code size! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing in-depth, just some minor comments having skimmed the code...
src/DNA/corlib/System/String.cs
Outdated
return tmp; | ||
} | ||
|
||
public static string Join<T>(string separator, T[] values) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if I'm missing something here, but Join<T>(string, T[])
isn't a standard member of string (see MS doc); the required method is Join(string, string[])
.
Also with the following method
src/DNA/corlib/System/String.cs
Outdated
#region IEquatable<string> Members | ||
|
||
[MethodImpl(MethodImplOptions.InternalCall)] | ||
extern public double ToDouble(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced this is a member of IEquatable<string>
;) or is even a public member of string at all...
src/DNA/native/src/Generics.c
Outdated
@@ -112,7 +120,7 @@ tMD_TypeDef* Generics_GetGenericTypeFromCoreType(tMD_TypeDef *pCoreType, U32 num | |||
memset(pTypeDef, 0, sizeof(tMD_TypeDef)); | |||
// Make the name of the instantiation. | |||
strcpy(name, pCoreType->name); | |||
strcat(name, "["); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[
is, I think, more correct that the proposed <
. The code here doesn't very accurately follow the way generic types are meant to be named, but .NET (real) does use [
, ]
as the generic container. See the second example in the MS docs here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chrisdunelm I know, but it's very confusing to see [
and ]
instead of <
and >
when types are printed out. It's also mainly cosmetic, so it shouldn't matter. And it's easy to change back if needed.
src/DNA/native/src/Heap.c
Outdated
moreRootsAdded = 1; | ||
} | ||
if (pType != types[TYPE_SYSTEM_WEAKREFERENCE]) { | ||
Heap_SetRoots(&heapRoots, pNode->memory, GetSize(pNode)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use pNode->totalSize
instead of GetSize()
?
src/DNA/native/src/Heap.c
Outdated
@@ -72,6 +72,8 @@ struct tHeapEntry_ { | |||
// unused | |||
U8 padding; | |||
|
|||
U32 totalSize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this just for speed? To avoid the cost of a GetSize()
call? It uses another 32-bits for every heap entry, which isn't disastrous, but isn't great either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chrisdunelm Thanks, I really appreciate your input. Yes, this is mainly for speed at the price of one more int per heap entry. It does help a little but it's not a huge win in release mode. I'll hide that behind ifdef so that it is configurable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ncave Have you profiled this? I'm just curious what difference it makes?
Mind you, the whole heap code really needs a rewrite to reduce per-object memory overhead, and to remove the need for the binary tree of allocations. Not really thinking of a generational GC, that would probably add to much code. This was something I vaguely intended to do, but never got around to.
The current heap code was just the simplest thing to get GC working quickly.
The very initial implementation did no GC at all. It was beautifully fast, but ran out of memory rather too quickly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chrisdunelm
Yes I have, it does make a difference in debug mode (obviously) and much smaller (but still measurable) difference in release mode. I've disabled it for now, but leaving it as configurable option.
The heap is great for what it is (non-generational) and it works well, as long as the alloc pressure is not too heavy, that's why I have increased the limits a bit. Obviously it would be nice to have a better one eventually, since it can eat a lot of the performance in heavy alloc scenarios.
LOL at the no GC allocator, seems to be a fad lately.
src/DNA/native/src/JIT_Execute.c
Outdated
@@ -106,7 +109,7 @@ tJITCodeInfo jitCodeGoNext; | |||
// Set the new method state (for use when the method state changes - in calls mainly) | |||
#define SAVE_METHOD_STATE() \ | |||
pCurrentMethodState->stackOfs = (U32)(pCurEvalStack - pCurrentMethodState->pEvalStack); \ | |||
pCurrentMethodState->ipOffset = (U32)(pCurOp - pOps) | |||
pCurrentMethodState->ipOffset = (U32)(pCurOp - pOps); \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tailing ; \
can be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
src/DNA/native/src/Heap.c
Outdated
@@ -473,8 +473,8 @@ HEAP_PTR Heap_Alloc(tMD_TypeDef *pTypeDef, U32 size) { | |||
} | |||
} | |||
|
|||
pHeapEntry = (tHeapEntry*)malloc(totalSize); | |||
memset(pHeapEntry, 0, totalSize); | |||
pHeapEntry = (tHeapEntry*)calloc(totalSize, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(1, totalsize)
?
@@ -0,0 +1,33 @@ | |||
// | |||
// IReadOnlyCollection.cs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why copy from mono instead of corefx?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No particular reason, this interface is the same either way. Perhaps one reason, the original DNA seem to be based on mono (although I can't be 100% sure), and the mono implementation is usually a bit smaller and simpler. In any case, I have refrained from copying too much, only the bare essentials to get things going.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, DNA originally used Mono code for classes that didn't require any integration into the C interpreter, and I didn't have the time/inclination to write from scratch. Clearly corefx didn't exist at the time!
Note that the classes I wrote were generally written to be small, rather than performant; the platform I originally wrote this for was very resource limited. Although this was all so long ago I can't really remember details.
Switching to using corefx code might now make some sense.
@SteveSandersonMS Rebased to latest. |
Love the project and eager to experiment with some ideas, but there are quite few outstanding PR's that contain a lot of changes. Could some of the PR-s be merged? |
@arpadbarta I know it's taking a while to merge this, and apologies for that. But please don't expect this repo to be a typical open-source project in a normal phase of maintenance. Instead, this repo is an experiment, not intended to have any particular level of support. Future plans are still being worked out. |
I like the sound of that, given the Blazor-Hackathon on the official |
I can't wait to see what this becomes. I am getting all giddy. XD |
I'm hoping collaboration with mono-wasm can speed things along by sharing resources, and advancing both experiments to an officially supported set of tools. Each project has a different architectural approach (e.g. client-side Razor vs. browser-targeting XAML), but so much of the underlying core and foundation would be common and sharable by both teams. |
And there's also this- dotnet/corert#4480 |
Microsoft really really really needs to take command of this, make it official and pull all of these initiatives together and get a cohesive whole that has 2 paths:
Pick which you want to use. In either case, both should automatically become a best of breed UWP application when targeted at the store with additional functionality available. And they should direct compile to iOS and Android as well. (without a requirement for a mac, meaning MS has to put the build and test stuff in the cloud for iOS and give it away free to devs so they have no reason to ever use a mac) |
Every single time I fire up a "React getting started" tutorial with typescript, webpack and redux, 5 minutes later I have stopped reading in frustration and I find myself back to this repo, gazing at the "what-could-be" future. I have done the "Todo List" sample in Blazor multiple times by now, demoing it to some colleagues, and every time it reminds me how easy and natural it feels writing in it. The timing is right. I believe that web assembly will even the playing field. It's time to seize this opportunity and make C# the go-to language for full stack web development. Please make this happen !! T_T |
Thanks @ncave for continuing to contribute to this! Just so you know, our hope is to move away from the DNA runtime shortly and replace it with a more full-featured one. If this works out (no guarantees yet though) then you could absolutely still continue to develop DNA (and there might indeed be demand for that as it might always be the smallest .NET runtime there is), but it would be good to move that over to another repo under your control. Does that sound OK? |
@SteveSandersonMS Thanks Steve, I figured you guys are up to something, and that's awesome, can't wait to see it. I already have another DNA development repo, that's where the updates are coming from. |
BTW In case you haven't seen it, I wrote a post to try and let more people know about DNA, see DotNetAnywhere: An Alternative .NET Runtime. There's also some interesting follow-up discussions, see HackerNews and /r/programming (including comments from @chrisdunelm) |
@mattwarren Of course I've seen it, great write-up, so much depth and detail. Thanks for the links! |
It is so great that this thing keeps moving! Can't wait to see it happen. While waiting I enjoyed browsing the following three repositories: WebAssembly is a new technology for running code at near-native speed in browsers. A group of folks on the .NET team worked together to see what WebAssembly might look like using CoreRT. They also issued a call for help dotnet/corert#4659 I have seen several pull requested merged in the last few days. So there is some action going on there. This project is intended to support running .NET code as an Electron Plugin, running code using the Mono runtime, side-by-side the V8 engine and to provide both C# API bindings to Electron's plugin API as well as the Web browser API. Last commit was 6 weeks ago. Seems like no public action going on here anymore. This project is a proof-of-concept aiming at building C# applications into WebAssembly, by using Mono and compiling/linking everything statically into one .wasm file that can be easily delivered to browsers This repository is owned by Laurent Sansonetti who works for Microsoft in Belgium. Steve and Laurent exchanged some emails and they tried to get it running with Blazor. Last commit was 10 days ago. |
In excited for the debugging support that blazor is doing. Hopefully it will be useful for the other frameworks. |
Moved to mono runtime! |
Not sure if this is correct place to ask. But after browsing ClientServerApp sample I noticed it uses bootstrap that uses jQuery. How does Blazor work with jQuery without memory leaks? What I mean is, lets say you have following <div id="foobar">
<a id="my">Click me</a>
</div>
<script>
$("#my").on("click", function () { console.log("clicked"); });
$("#my").data("foo", "bar");
$("#foobar").html("");
</script> In this situation jQuery automatically removes events and element data attached to #my from memory. But if you replace $("#foobar").html("") with document.getElementById("foobar").innerHTML = "" it doesn't remove them and this causes memory leak. Also about Bootstrap modal that is used in sample, when you open a bootstrap modal it appends elements to end of body that need additional cleanup to remove completely. With Bootstrap 3 it's harder to clean up. With Bootstrap 4 you can use .modal('dispose'). How can you remove jQuery components properly with Blazor? This applies to vanilla JavaScript components as well that need cleanup. Angular 2+ has ngOnDestroy, React has componentWillUnmount and VueJS has beforeDestroy events for that. |
Wish list: |
Here are a few fixes and updates to the corlib and JIT:
I see you added CLR debugging in the browser, that's awesome!