-
Notifications
You must be signed in to change notification settings - Fork 0
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 marshaler registration source generator #2
Wasm marshaler registration source generator #2
Conversation
Add test coverage for date marshaling Add uri test Disable some log statements Returning structs works Returning custom classes works Add null checks to describe_value Remove invalid cwrap Add trimming annotations Restore old benchmark html Reuse result and exception roots Cleaner codegen Disambiguate converters in devtools Transition back to static methods, add signature checks and more error handling Implement a simple bound method cache so that automated tests don't exhaust the scratch root buffer; make the scratch root buffer smaller Fix a LMF leak Fix C warnings Hard-coded table of marshalers is no longer needed Shorter wrapper function names for better stacks Add linker exclusions for custom marshalers Hack around bug in linker/runtime that causes System.Uri forwarder to break Rework class lookup Support passing numeric values as DateTime Transition some null returns to asserts Clean up conditionals Don't use ISO strings Change pre/post filter syntax to require an explicit "return" so it can contain multiple statements Allow ToJavaScript to accept a pointer instead of an 'in' reference Support managed pointer return/parameter types in more places Add marshal_type for pointers Add tests verifying that you can marshal structs by address and then unpack them Initial gc safety work Fix formatting rule Rename pre/post filters Fix type error in driver.c remove _pick_result_chara_for_marshal_type Add library build descriptor to ensure marshalers are not stripped when the BCL is trimmed Attempt to fix the linker stripping test code Better version of no-configured-marshaler warning Annotate SetupJSContinuation Annotate safehandle APIs and add null checks Update targets file to pass custom marshaler msbuild items through to the helix proxy project (requires another change to work) Add tests for Task and ValueTask marshaling Fix unboxing for generic structs Rebase cleanups Correct datetime test to use UTC for comparison Eliminate use of MONO. and BINDING. in closures Optimize out a js->c call Normalize some APIs to take MonoType instead of MonoClass Repair merge damage Address PR feedback Move some types around Repair merge damage Type system fixes
…closure keys more efficiently
Maybe in generated code you should do |
@kg The Or @pavelsavara has the idea that I could generate a header for mono_wasm_register_custom_marshaler in the MarshalerInitializer. |
Ok, that doesn't work with
|
I will start a new "public" JavaScript library here, so the attribute and generator can be consumed in other projects. |
My understanding is we already have a tasks assembly that contains source generators for other wasm-related things, maybe we can put this there? |
The problem here is that The generator itself can be anywere. |
That is good opportunity to draft public API assembly here. And learn how to make it public and included with runtime. |
Adding a public API will require review, which I think we want to avoid at this point.
I think we can do like other new features do and match the attribute by name so that an assembly can provide the attribute itself until we make it public (if ever). For the registration function, we can do a similar trick since icalls are matched by name - we can just put the icall in the assembly next to the generated module initializer.
On November 23, 2021 12:39:41 PM PST, Pavel Savara ***@***.***> wrote:
That is good opportunity to draft public API assembly here. And learn how to make it public and included with runtime.
We could later move things from private interop namespace, which we agree that have nice enough API.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#2 (comment)
- kg (mobile)
|
Yes, this is doable.
I tried it, but it did not work. I'll ping you tomorrow with the code I had. |
I can even generate the attribute itself... |
- Source generator project. - Base for finding classes annotated with MarshalerAttribute.
Since the attribute is generated, we don't get a semantic information about it, and we need to match the attribute by name only.
Thinking aloud. Maybe we could have multiple ways.
The problem with 2) is that code of marshaller itself is using the marshalled type. Could we somehow tell the linker to ignore it ? Or tell the runtime to late-bind it ?
Separate the registration from the implementation ? |
https://www.codeguru.com/cplusplus/arranging-custom-marshaling-with-p-invoke/ We could also have the attribute at the method which wants to use custom marshaller. In the same way as PInvoke. Being able to customize marshaller locally, not globally is probably right approach anyway. |
Event if we annotate the type with the attribute and marshaler in it, the linker won't be able to trim it when the type is used somewhere in the app, although it's not used in the interop. From the linker perspective it's the solution to apply the attribute on the interop method itself. Because if the method is not used, it will be trimmed and so the marshaler type. But it is kind a annoying from user perspective. Is there a way to determine which method is used "in the interop"? |
FYI, this old API explicitly was called out as a bad design when we previously did API review for JS custom marshalers. So we shouldn't be using it as an example. |
221f4c4
to
b23df27
Compare
8afdc4a
to
0deb1c2
Compare
I squashed+rebased this and shoved it at https://github.com/kg/runtime/tree/maraf-sourcegen, will be forking off that to mess with integrating a source generator into my PR since it has a C#-based code generator now. |
From testing this, the removal of the .xml files breaks it because the linker is now stripping all of the statics out of the marshalers. |
fd1dc81
to
85eb349
Compare
* Initial implementation for contract customization fix build errors Move converter rooting to DefaultJsonTypeInfoResolver so that it can be used standalone Fix ConfigurationList.IsReadOnly Minor refactorings (#1) * Makes the following changes: * Move singleton initialization for DefaultTypeInfoResolver behind a static property. * Consolidate JsonSerializerContext & IJsonTypeInfoResolver values to a single field. * Move reflection fallback logic away from JsonSerializerContext and into JsonSerializerOptions * Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs * remove testing of removed field Simplify the JsonTypeInfo.CreateObject implemenetation (#2) * Simplify the JsonTypeInfo.CreateObject implemenetation * Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs * Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs Co-authored-by: Krzysztof Wicher <[email protected]> Co-authored-by: Krzysztof Wicher <[email protected]> Tests and fixes for JsonTypeInfoKind.None TypeInfo type mismatch tests Allow setting NumberHandling on JsonTypeInfoKind.None test resolver returning wrong type of options JsonTypeInfo/JsonPropertyInfo mutability tests rename test file Move default converter rooting responsibility behind DefaultJsonTypeInfoResolver (dotnet#3) * Move default converter rooting responsibility behind DefaultJsonTypeInfoResolver * address feedback Add simple test for using JsonTypeInfo<T> with APIs directly taking it fix and tests for untyped/typed CreateObject uncomment test cases, remove todo More tests and tiny fixes Add a JsonTypeInfoResolver.Combine test for JsonSerializerContext (dotnet#4) * Fix JsonTypeInfoResolver.Combine for JsonSerializerContext * Break up failing test Fix simple scenarios for combining contexts (dotnet#6) * Fix simple scenarios for combining contexts * feedback JsonSerializerContext combine test with different camel casing Remove unneeded virtual calls & branching when accessing Get & Set delegates (dotnet#7) JsonPropertyInfo tests everything minus ShouldSerialize & NumberHandling Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs throw InvalidOperationException rather than ArgumentNullException for source gen when PropertyInfo.Name is assigned through JsonPropertyInfoValues tests for duplicated property names and JsonPropertyInfo.NumberHandling Add tests for NumberHandling and failing tests for ShouldSerialize disable the failing test and add extra checks disable remainder of the failing ShouldSerialize tests, fix working one Fix ShouldSerialize and IgnoreCondition interop Add failing tests for CreateObject + parametrized constructors Fix CreateObject support for JsonConstructor types (dotnet#10) * Fix CreateObject support for JsonConstructor types * address feedback Make contexts more combinator friendly (dotnet#9) * Make contexts more combinator friendly * remove converter cache * redesign test to account for JsonConstructorAttribute * Combine unit tests * address feedback * Add acceptance tests for DataContract attributes & Specified pattern (dotnet#11) * Add private field serialization acceptance test (dotnet#13) * tests, PR feedback (dotnet#14) * PR feedback and extra tests * Shorten class name, remove incorrect check (not true for polimorphic cases) * Make parameter matching for custom properties map property Name with parameter (dotnet#16) * Test static initialization with JsonTypeInfo (dotnet#17) * Fix test failures and proper fix this time (dotnet#18) * Fix test failures and proper fix this time * reinstate ActiveIssueAttribute * PR feedback and adjust couple of tests which don't set TypeInfoResolver * fix IAsyncEnumerable tests * Lock JsonSerializerOptions in JsonTypeInfo.EnsureConfigured() Co-authored-by: Eirik Tsarpalis <[email protected]> Co-authored-by: Eirik Tsarpalis <[email protected]>
…tnet#87189) This fixes a startup crash on Big Sur: > error: * Assertion at /Users/runner/work/1/s/src/mono/mono/utils/mono-hwcap-arm64.c:35, condition `res == 0' not met Because sysctl can't find some of these options: $ sysctl hw.optional.armv8_crc32 hw.optional.armv8_crc32: 1 $ sysctl hw.optional.arm.FEAT_RDM sysctl: unknown oid 'hw.optional.arm.FEAT_RDM' $ sysctl hw.optional.arm.FEAT_DotProd sysctl: unknown oid 'hw.optional.arm.FEAT_DotProd' $ sysctl hw.optional.arm.FEAT_SHA1 sysctl: unknown oid 'hw.optional.arm.FEAT_SHA1' $ sysctl hw.optional.arm.FEAT_SHA256 sysctl: unknown oid 'hw.optional.arm.FEAT_SHA256' $ sysctl hw.optional.arm.FEAT_AES sysctl: unknown oid 'hw.optional.arm.FEAT_AES' Full stack trace: * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 2.1 * frame #0: 0x0000010ef37560 libmonosgen-2.0.dylib`monoeg_assertion_message frame #1: 0x0000010ef375cc libmonosgen-2.0.dylib`mono_assertion_message + 32 frame #2: 0x0000010ef40d6c libmonosgen-2.0.dylib`mono_hwcap_arch_init + 544 frame dotnet#3: 0x0000010ef54bd8 libmonosgen-2.0.dylib`mono_hwcap_init + 72 frame dotnet#4: 0x0000010ee14dc0 libmonosgen-2.0.dylib`parse_optimizations + 52 frame dotnet#5: 0x0000010edbed48 libmonosgen-2.0.dylib`mono_init frame dotnet#6: 0x0000010ee18968 libmonosgen-2.0.dylib`mono_jit_init_version frame dotnet#7: 0x0000010f48a300 libxamarin-dotnet-debug.dylib`xamarin_bridge_initialize + 216 frame dotnet#8: 0x0000010f4900a4 libxamarin-dotnet-debug.dylib`xamarin_main + 376
…tnet#90436) * Enable IL trim for WASM by default * Make ILStrip available for local build * Make all calling another method go through the logic to see if it could call an AOT'ed version of it before trying to interp compile it * Add back accidentally removed line of code * Update test to accommodate IL trim with WASM AOT * Move jit_call_can_be_supported to mini-runtime, so it doesn't depends on the value of mono_use_interpreter * Update var name * Attempt to fix rebuild test failures * Attempt to fix the file open issue with unicode on windows * Attempt to fix unicode issue #2 * Enable g_fopen to have the capability of handling opening files with unicode name on all platforms * Add comment * Update comment * Fix file indentation format * Check if string contains non-ascii char * Remove unused callback * Remove redundant comment * Update method name * Fixed some method not found issues and remove the optimization for g_fopen * Fix tailcall * Disable tailcall optimization when calling a trimmed method * Free method header * Fix windows build error * Free method header at the correct locatioin * Fix the condition of skipping tailcall * Fix test failure * Move JIT/AOT call invoke away from MINT_CALL, as it is not needed there. * Fix virtual tail call * Address review feedback * Put the trimmed assemblies in a new folder, output an updated list of assemblies and update _WasmAssembliesInternal with the new list * Put trimmed assemblies in IntermediateOutputPath * Remove TrimmedAssemblies * Create trimmed assembly folder before the parallel run * Try to fix the issue with missing item * Fix parallelism issue * Only start the trim when the assembly is newer than the output * Add assembly item to the list, when * Add some logging * Fixed runtimeconfig.json file path issue and disabed failed tests * Update parameter name * Fix wasi build * Use the correct parameter * Fix runtime test failure * WasmAppBuilder: runtimeconfig.json path can be null * cleanup * ILStrip: fix typo in id name * Cleanup * ILStrip: ensure output assemblies are in the same order as the input. This is required for incremental builds. * more cleanup * Re-enable disabled tests * Change the default value for WasmStripILAfterAOT to false * Fix the issue with changing the value of WasmStripILAfterAOT between incremental builds * Move the location of file deleting * Use WasmAssembliesFinal or ResolvedFileToPublish during publish in WasmSDK * Copy metadata in AOT compiler and when creating WasmAssembliesToBundle from ResolvedFileToPublish * Add _WasmSatelliteAssemblies to WasmAssembliesFinal * Add a wasm template test * Include all non-dll ResolvedFileToPublish for ComputeWasmPublishAssets * Add a blazor template test * Address review feedback * Update src/mono/wasm/Wasm.Build.Tests/Templates/WasmTemplateTests.cs Co-authored-by: Ankit Jain <[email protected]> * Update parameter name * Update usage of parameter * Revert EmccCompile change * MonoAOTCompiler: revert rebuilding when WasmStripILAfterAOT value changes * Compress -> GZipCompress, and cleanup * rework the tests - to support webcil case - run blazor app - add cases for the default setting, and the opposite - cleanup * ILStrip.cs: Emit a message about stripping to make it obvious to the user * WasmApp.targets: update comment * Change default value to false and update test --------- Co-authored-by: Ankit Jain <[email protected]> Co-authored-by: Marek Fišera <[email protected]>
Fixes dotnet#95367. Relevant part of the JitDump: ``` Using `if true` assertions from pred BB02 Assertions in: #1 fgMorphTree BB04, STMT00021 (before) [000070] DA--------- * STORE_LCL_VAR ubyte V10 tmp9 [000057] ----------- \--* CAST int <- ubyte <- int [000006] ----------- \--* EQ int [000004] ----------- +--* LCL_VAR ref V02 tmp1 (last use) [000055] H---------- \--* CNS_INT(h) ref 'Frozen EmptyPartition`1<Int32> object' Assertion prop for index #1 in BB04: [000006] ----------- * EQ int GenTreeNode creates assertion: [000070] DA---+----- * STORE_LCL_VAR ubyte V10 tmp9 In BB04 New Local Constant Assertion: V10 == [0000000000000001], index = #2 fgMorphTree BB04, STMT00021 (after) [000070] DA---+----- * STORE_LCL_VAR ubyte V10 tmp9 [000055] H----+----- \--* CNS_INT(h) int ``` The JitDump is unfinished because the compiler crashes when trying to dump the last line. Clearly, the `CNS_INT` is no longer a handle at that point because we just bashed it to a constant 1.
…tnet#95292) * Add IndentText json option * Add IndentText for json source generator * Add tests * IndentText must be non-nullable * Improve performance * Add extra tests * Cleanup * Apply suggestions from code review Co-authored-by: Eirik Tsarpalis <[email protected]> * Fixes following code review * Fixes following code review #2 * Add tests for invalid characters * Handle RawIndent length * Move all to RawIndentation * Update documentation * Additional fixes from code review * Move to the new API * Extra fixes and enhancements * Fixes from code review * Avoid introducing extra fields in JsonWriterOptions * Fix OOM error * Use bitwise logic for IndentedOrNotSkipValidation * Cache indentation options in Utf8JsonWriter * Add missing test around indentation options * New fixes from code review * Update src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.cs * Add test to check default values of the JsonWriterOptions properties * Fix comment --------- Co-authored-by: Eirik Tsarpalis <[email protected]>
* [wasm] Bump emscripten to 3.1.56 * Replace Module.asm with Module.wasmExports Module.asm was removed, use wasmExports instead. Context: emscripten-core/emscripten#19816 * Updates for .native.worker.js -> mjs rename Context: emscripten-core/emscripten#21041 * Update deps * Add general testing feed * Update mode deps * Update path * Use current python packages for now, we don't have newer ones The current names 3.1.34 are new enough * Keep using llvm 16 for runtime and aot compiler * Add -Wno-pre-c11-compat only for browser * Temporarily disable version checks to get further * Temporarily disable version checks to get further #2 * Disable -Wunused-command-line-argument * Update emsdk deps * Update icu dependency * Revert "Temporarily disable version checks to get further #2" This reverts commit 3f8834f. * Revert "Temporarily disable version checks to get further" This reverts commit fe1e5c6. * Fix emsdk check We use system python on osx too * Workaround wasm-opt crash * Workaround wasm-opt crash * Workaround wasm-opt crash * Fix WBT test * Feedback * Update ICU dependency * Update emscripten deps * Revert "Workaround wasm-opt crash" This reverts commit 200cf3b. * Revert "Workaround wasm-opt crash" This reverts commit 4530edf. * Revert "Workaround wasm-opt crash" This reverts commit 3593c41. * Increase tests timeout * Show test progress * Increase MT library tests timeout * Disable WBT tests with SkiaSharp * Increase helix tests timeout on browser * Increase WBT timeout * Increase initial heap sizes * Fix mono_wasm_load_runtime cwrap signature Fixes: `Uncaught ExitStatus: Assertion failed: stringToUTF8Array expects a string (got number)` * Enable XunitShowProgress for threading tasks tests * Try to reduce number of parallel AOT compilations To check whether it will improve memory issues on CI * Use new docker image for helix/windows tests * Revert "Try to reduce number of parallel AOT compilations" This reverts commit 5d9a6d2. * Reduce the timeouts * Reduce intitial heap size * use active issues for MT * Remove testing channel from nuget config, update deps * Update emsdk and icu dependencies --------- Co-authored-by: Larry Ewing <[email protected]> Co-authored-by: pavelsavara <[email protected]>
* bug #1: don't allow for values out of the SerializationRecordType enum range * bug #2: throw SerializationException rather than KeyNotFoundException when the referenced record is missing or it points to a record of different type * bug dotnet#3: throw SerializationException rather than FormatException when it's being thrown by BinaryReader (or sth else that we use) * bug dotnet#4: document the fact that IOException can be thrown * bug dotnet#5: throw SerializationException rather than OverflowException when parsing the decimal fails * bug dotnet#6: 0 and 17 are illegal values for PrimitiveType enum * bug dotnet#7: throw SerializationException when a surrogate character is read (so far an ArgumentException was thrown)
MarshalerAttribute
,Interop.Runtime.InvokeJS
andMarshalerInitializer
.[Marshaler(typeof(...))]
syntax.MarshalerAttribute.g.cs
Interop.Runtime.g.cs
MarshalerInitializer.g.cs sample
I'm not sure about project name and location, please suggest better for both.