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

Add support for .NET 7/C# #29

Merged
merged 37 commits into from
Aug 15, 2022
Merged

Add support for .NET 7/C# #29

merged 37 commits into from
Aug 15, 2022

Conversation

inkeliz
Copy link
Owner

@inkeliz inkeliz commented Aug 5, 2022

No description provided.

@inkeliz
Copy link
Owner Author

inkeliz commented Aug 5, 2022

Still missing:

@inkeliz
Copy link
Owner Author

inkeliz commented Aug 5, 2022

There's also two issues:

  • C# can't have fields with the same name of the Class, such as:
public struct MyName {
    public uint MyName = 0;
}

That forces the generator to include _ in the name, transforming the field to _MyName. That can be undesirable in must cases. I think the best approach is fixing #17, then emitting warnings when such collisions happens and maybe just append _ to conflicted names.

  • C# can't hold more than 2GB on array:

The List<T> can only use int as key/index, and that is a limiting factor. Also, far I understand, any managed object can't use more than 2GB. However, unmanaged doesn't have such limits.

Because of that I'm thinking to move all types of array to Karmem.Slice, since it's just one pointer to the data. The 2GB restrictions doesn't apply to

@inkeliz
Copy link
Owner Author

inkeliz commented Aug 7, 2022

Currently, the test is faling due to out-of-memory. That issue is already know: SteveSandersonMS/dotnet-wasi-sdk#11.

However, that is might a hint of poor performance and the lack stack-reuse. The code should re-use the space and should prevent additional allocation. It can be either an lack of C# optimization or bug in the current Karmem code. More experimentation soon.

dotnet/Karmem.cs Outdated Show resolved Hide resolved
__attribute__((export_name("InputMemoryPointer")))
uint32_t InputMemoryPointer() {
MonoObject* err;
MonoObject* res = mono_wasm_invoke_method (_ID_InputMemoryPointer, _CLASS_Benchmark, NULL, &err);
Copy link
Owner Author

Choose a reason for hiding this comment

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

How release that pointer? 🧐

@inkeliz
Copy link
Owner Author

inkeliz commented Aug 7, 2022

Looking into Span internals, seems that is optimized by JIT, which might explains why karmem.Slice is ~5ms slower. I'm not C# expert, but sounds like there's some internal code optimized and I'm not sure if there's any equivalent of go:linkname from Go on C#, allowing to call internal functions directly.

Currently, benchmark for Vec3 is ~75ms (and ~70ms using Span), on Ryzen 3900X (M1 tests soon).

That still significantly slower than Go, Zig, C, AS and even Swift (with #11 optimization). I'm not sure what I can do to optimize it, and sounds like issues on C# runtime directly.

@inkeliz
Copy link
Owner Author

inkeliz commented Aug 8, 2022

Ok. Compiling without -c Release reveals the issue, and it's releated to the lack of GC (or lack of GrowCalls):

[wasm_trace_logger] * Assertion at /home/runner/work/dotnet-wasi-sdk/dotnet-wasi-sdk/modules/runtime/src/mono/mono/metadata/sgen-stw.c:77, condition `info->client_info.stack_start >= info->client_info.info.stack_start_limit && info->client_info.stack_start < info->client_info.info.stack_end' not met

    main_wasi_fuzz_test.go:126: wasm error: unreachable
        wasm stack trace:
                ._Exit(i32)
                .exit(i32)
                .wasm_trace_logger(i32,i32,i32,i32,i32)
                .eglib_log_adapter(i32,i32,i32,i32)
                .monoeg_g_logstr(i32,i32,i32)
                .monoeg_g_logv_nofree(i32,i32,i32,i32) i32
                .monoeg_assertion_message(i32,i32)
                .mono_assertion_message(i32,i32,i32)
                .update_current_thread_stack(i32)
                .sgen_client_stop_world(i32,i32)
                .sgen_stop_world(i32,i32)
                .sgen_perform_collection_inner(i32,i32,i32,i32,i32)
                .sgen_perform_collection(i32,i32,i32,i32,i32)
                .sgen_ensure_free_space(i32,i32)
                .sgen_los_alloc_large_inner(i32,i32) i32
                .sgen_alloc_obj_nolock(i32,i32) i32
                .mono_gc_alloc_vector(i32,i32,i32) i32
                .mono_array_new_specific_internal(i32,i32,i32,i32) i32
                .mono_array_new_specific_checked(i32,i32,i32) i32
                .interp_exec_method(i32,i32,i32)
                .interp_runtime_invoke(i32,i32,i32,i32,i32) i32
                .mono_jit_runtime_invoke(i32,i32,i32,i32,i32) i32
                .do_runtime_invoke(i32,i32,i32,i32,i32) i32
                .mono_runtime_try_invoke(i32,i32,i32,i32,i32) i32
                .mono_runtime_invoke(i32,i32,i32,i32) i32
                .mono_wasm_invoke_method_ref(i32,i32,i32,i32,i32)
                .mono_wasm_invoke_method(i32,i32,i32,i32) i32
                .KBenchmarkDecodeObjectAPI(i32)

@inkeliz inkeliz force-pushed the lang-dotnet branch 5 times, most recently from 29c5431 to 35e31d3 Compare August 9, 2022 19:04
@inkeliz
Copy link
Owner Author

inkeliz commented Aug 10, 2022

The tests are passing on TCP/Native, the Fuzzing is falling on WASM. However, the Flatbuffers is also failling on WASM, while working on TCP/Native.

I'll assume that it's an issue of DotNet-WASI-SDK (SteveSandersonMS/dotnet-wasi-sdk#11), however it might be one issue from Karmem itself.

inkeliz added 15 commits August 15, 2022 23:11
This patch adds support for .NET 7.
That change makes possible to use uint/ulong as index-key. However,
since C# doesn't have `usize`, we need to use `ulong`.
Before that patch, Karmem.Slice doesn't support other value
rather than Karmem.IViewer.
Before that patch, the value was retrieved by copy. That prevents
modifications of non-primitive values doesn't reflects to the
original List<T>.
That implements the Reset function, which is present for all
generated code.
Before that patch, new allocations are not cleared
That changes makes possible to compile to WASM with the standard
benchmark exported functions.
Now it's possible to compile and test/bench with `make dotnet`.
… instead of opaque pointers

Previously, the Viewer struct is one opaque pointer to the data. Now,
that was changed to "hold" the content and be able to use unsafe to
cast it directly, instead of having one temporary variable.

The performance improves from ~104ms to ~89ms.

The struct uses `Unsafe.AsPointer(this)` to get their own pointer.
Previously, we remain check for implementation of IViewer. However,
since last patch it's no longer necessary and we can remove the
interface.

That also improves the performance from ~89ms to ~75ms.
That reverts the #37a7496 commit, however Viewers is also supported.
inkeliz added 22 commits August 15, 2022 23:23
Now, the generated code uses Span.
Previously, the size of the string was wrongly calculated and defined.
That change makes clear that Viewers are not suppose to be modified.
Some languages are failing the fuzzing for reasons outside our control.
That makes possible to test without need WebAssembly
To compile for WASM you need to provide `-p:IS_WASM=true` building tags
Previously, the Flatbuffers and Karmem are included, even
when not used.
@inkeliz inkeliz merged commit 3b20cee into master Aug 15, 2022
@inkeliz inkeliz mentioned this pull request Aug 23, 2022
@inkeliz inkeliz deleted the lang-dotnet branch August 29, 2022 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant