Fix json code gen #344
Merged
Fix json code gen #344
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In (non-palsuite) product code, `_wcslwr` is only used within PAL inside `_wcslwr_unsafe()` method, which is exposed as `_wcslwr_s` for PAL consumers. PR inlines the usage of `_wcslwr` in `_wcslwr_unsafe` and fixes up PAL tests.
Before this change, an InterpFrame contained 3 regions of data : args + locals, valuetype stack, execution stack. Each entry on the execution stack was a stackval structure. The space for valuetypes, was allocated separately, since they have various sizes. When pushing a valuetype on the stack, we allocated first the space for it on the vtstack and then pushed the address of the region on the execution stack. This change merges the execution stack with the valuetype stack, meaning we push now variable sized data on the execution stack. In order to keep track of the current stack location, whenever we push a type on stack, during transform phase, we also keep track of the offset where this value will reside on stack, as well as the size it occupies. All callsites need to be informed how much they need to pop the stack for the arguments. While called code can access this space normally (the args are special locals belonging to the frame and are accessed directly as such), external code needs a new mechanism to detect each argument for a given frame. This is achieved with the lazily initialized arg_offsets array on an InterpMethod. The method doesn't need to be compiled for this array to be correctly initialized. Why : - this simplifies handling of valuetypes, their storage follows the same rules as a normal objref/primitive type - removes the common use of the vt_sp variable. The compiler no longer needs to reserve it in a register during the switch loop, we no longer need to save it with each call. The sp and ip become now the only variables describing the execution state in a method. - the flow of the data on the execution stack is well behaved now (with the exception of a few opcodes that update directly based on the stack offset). We were using the vtstack for some magic storage (MINT_VTRESULT for example) - this makes it such that the stack offset of every value is easily known at compile time, making it possible to completely drop the execution stack approach, and have every opcode have a unique dreg and a list of sregs (which are mapped to a certain stack offset). This will enable more advanced optimizations during compile stage. Co-authored-by: BrzVlad <[email protected]>
This reverts commit 3a4298c.
* Statically linking coreclr and clrjit in single file host. * setting g_hmodCoreCLR on Unix * System.Globalization.Native.lib must build with coreclr to be linkable with it * Always use system unwind libs on FREEBSD * no DllMain when coreclr is statically linked * Handle cases when coreclr configuration is different from libraries * Adding and using PAL_GetPalHostModule
… (#43625) * simplify SslStream_StreamToStream_Alpn_NonMatchingProtocols_Fail test * feedback from review
* Improve annotations for XLinq classes taking params object[] * annotate ref * address Jozkee's feedback * Fix XStreamingElement ctor to take nullable content
Propagate GTF_CALL if needed in GT_LIST. Use gtNewListNode. Ignore test for Mono
…g stops a thread from processing work (#43840)
…isions (#43836) * Add IDictionary_Generic_Tests test for multiple values with hash collisions * Update src/libraries/System.Collections.Concurrent/tests/ConcurrentDictionary/ConcurrentDictionary.Generic.Tests.cs Co-authored-by: Eirik Tsarpalis <[email protected]> * Update src/libraries/System.Collections.Concurrent/tests/ConcurrentDictionary/ConcurrentDictionary.Generic.Tests.cs Co-authored-by: Eirik Tsarpalis <[email protected]>
* Avoid several WildcardBindForConnectIfNecessary allocations on each connect
* Rewrite Socket.ConnectAsync for DNS with async/await
Rips out all of the APM code that was previously used to implement this and replaces it with {Value}Task-based async/await implementations.
Intrinsify BitOperations.PopCount for constant input
…operators (#43567) Fold "(X op C1) op C2" to "X op (C1 op C2)"
The return type validation was rejecting cases when the method being overriden had canon type in its generic arguments. This change fixes the problem by using parent method type instantiation for constructing the SigTypeContext in such case. It also adds a regression test.
…nal PR (#43161) * Add exception case xml comment for ExecutionContext.Restore * Add some info about how named mutexes work on Unixes into code from the PR
- Fixed an assertion failure. `WorkerCounter` shouldn't be used in the native thread pool implementation when the portable thread pool is enabled (all the counts will be zero), fixed a missed case in `GetAvailableThreads`. - The native implementation uses a smaller max default worker thread count by default on 32-bit platforms, allowing configured values to go beyond that. Fixed the portable thread pool implementation to do similar, instead of limiting the max including for configured values.
* Prototype for runtime tests running with Android * Conditionally collect app dependencies * Switch android sample from publish to build * Switch Android sample from publish to build * Modify AndroidTestRunner * Only build and test one test * Clean up some changes * Update new run test script path and update AndroidTestRunner to use files under CoreRoot * Add Helix configuration for Android_x64 * Disable test which replies on coreclr System.Private.CoreLib.dll * Fixed format and removed irrelevant parameters * Rmoved unused parameters and fixed comment * Revert my AndroidAppBuilder related changes to prepare for getting Egor's AndroidAppBuilder change * Fixed rebase and merge error * Update AndroidTestRunner * Use the AndroidAppBuilder in the publish folder * Update runtime test Android App creation command * Temporarily disable all jobs except for the Android one * Fix up job running config * Fixed AndroidTestRunner * Get Main function return code after running android app * Fix for skipping building native tests * Fix helix project path, due to recent coreclr test related files relocation. * Fix android app running commands * Add missing helixType * Update HelixRuntimeRid * Fix inconsistency * Create Android app during build preiod. * Adjust the way to run tests * Temporary hack before AndroidAppBuilder could accept multiple assemblies * Use xharness instead of adb to run tests on Androids and disable cmake for Android. * Cleanup unnecessary changes * Include xharness Cli * Use the right xharness command * Copy runtime assemblies, since the copy logic got removed from AndroidAppBuilder * Save Android apps in CORE_ROOT dir and adjust BuildAndroidApp target due to rebase from master * Minor cleanup * Enable all runtime tests and disable tests caused build failure * Copy over test helper assemblies and add failing tests to issues.targets * Remove redundant comment * Revert previous commit * Only sending apk files to helix machine * Delete OutputDir after finishing building the app * Copy whole Core_Root dir to helix machine. * Extend timeout threshold * Extend timeout threshold for each test and each test collection * Display Android app running logs when tests fail * Android tests are required to run in sequential * Check if log file exists before using it. * Exclude two out-of-memory tests * Fixed merge error * Shorten the timeout threshold * Enable all the lanes * Fix indentation * Fixed a few issues from review feedback * Update src/tests/run.proj Co-authored-by: Alexander Köplinger <[email protected]> * Only bail out from the bash script when erroring out for andriod * Extend timeout threshold, since the run could take a long time or a short time on CI. * Fixed merge error * Filter out tests using more memory than allowed by helix machine Co-authored-by: Alexander Köplinger <[email protected]>
…#43786) * Add backward file from iana to handle backward compatibility in tz data * add tests to check for backwards compatibility
* m_dwForbidSuspendThread * Remove some fibers stuff * g_TrapReturningThreads * g_pGCSuspendEvent * m_pThreadAttemptingSuspendForGC * refactor SuspendRuntime into just one loop * Suspend iteration after updating hijacks should "observeOnly" - no point to update hijacks again right away. * avoid retrying to suspend threads that are already in redirected state. * PR feedback * "premptive" - common typo apparently * missing profiler notification on suspend end and `DisableStressHeap` stuff * indentation and infinite loop style * Removed GTF_DONT_CSE flag on `g_TrapReturningThreads` load in GCPoll and added comment explaining why. * reverted an accidentally removed assert * removed SWITCHOUT_HANDLE_VALUE entirely - not different from INVALID_HANDLE_VALUE now * some cleanups and more comments when non-GC suspension yields to GC * Few tweaks for the concerns discussed in the PR * On Server GC `pCurThread` is NULL. Need to fix an ASSERT to account for the possibility of NULL.
* Disable qcalls on wasm. Treat them as normal pinvokes. This is needed because they are stored in a separate table, so they cannot be linked out the same way as pinvokes/icalls. * Update tools-local/tasks/mobile.tasks/WasmAppBuilder/PInvokeTableGenerator.cs Co-authored-by: Ryan Lucia <[email protected]> * Update tools-local/tasks/mobile.tasks/WasmAppBuilder/PInvokeTableGenerator.cs Co-authored-by: Ryan Lucia <[email protected]> Co-authored-by: Ryan Lucia <[email protected]>
* Change Task.FromResult to use same task cache as async methods Task.FromResult today always creates a new task. This leads developers that are aware of this to then create their own caches for common values, typically 0, true, and false, to avoid task allocations for those common values. But we already have such values cached internally, used for async method return values. We can just use the same cache for Task.FromResult. * Address PR feedback
* Fix invalid assert in StreamBuffer
This can be hit by calling code and thus shouldn't be an assert. As an assert it prevents testing.
* Add ConnectedStreams.CreateUnidirectional
* Add initial set of Stream conformance tests
These primarily focus on "connected streams", ones that can be arranged to communicate with each other in a producer/consumer pattern or as a bidirectional communication mechanism, e.g. NetworkStream, PipeStream, SslStream wrapped around a NetworkStream, FileStream created around pipes, etc. Later we can add more tests focused on standalone streams, e.g. FileStream created for an on-disk file, MemoryStream, etc.
* Add ConnectedStreams tests
These are currently helpers used by many other tests. At some point they could become public API as well.
* Fix NetworkStream argument names
Technically a breaking change, but the current divergence from the base Stream class is a bug, and bringing them into sync means argument exceptions that emerge from the derived type make sense when used via the base type, as well as then being able to use shared validation logic across all streams (subsequent to these changes).
* Add stream conformance tests for NetworkStream
* Add stream conformance tests for QuicStream
* Fix several CryptoStream behaviors
1. Flushing a stream that wraps another stream for writing should always flush that underlying stream, even if no additional data was written as part of the flush.
2. Argument validation should validate buffers are not null rather than null ref'ing on a null buffer.
3. Checks for the CryptoStream mode should come after argument validation.
* Add stream conformance tests for CryptoStream
* Fix FileStream argument names
Technically a breaking change, but the current divergence from the base Stream class is a bug, and bringing them into sync means argument exceptions that emerge from the derived type make sense when used via the base type, as well as then being able to use shared validation logic across all streams (subsequent to these changes).
* Fix BufferedStream argument names
Technically a breaking change, but the current divergence from the base Stream class is a bug, and bringing them into sync means argument exceptions that emerge from the derived type make sense when used via the base type, as well as then being able to use shared validation logic across all streams (subsequent to these changes).
* Add stream conformance tests for FileStream
Specifically when used in a connected fashion, wrapped around an anonymous or named pipe.
* Add stream conformance tests for BufferedStream
Specifically when used in a connected fashion, wrapped around some other connected stream.
* Add stream conformance tests for SslStream and NegotiateStream
* Fix PipeStream.Flush to not fail on readable streams
Consumers may expect Stream.Flush to be a nop if the stream is readable only, but PipeStream.Flush is throwing in that case. Stop doing that.
* Add stream conformance tests for PipeStream
* Fix several BrotliStream behaviors
1. When passed a null buffer, it's throwing an exception with the argument name "array", even though the parameter's name is "buffer".
2. Even if there's no data written as part of the Flush{Async} call on a writeable stream, it should be calling flush on the wrapped stream.
* Fix several DeflateStream (and friends) issues
1. DeflateStream.Flush{Async} when writing needs to always flush the underlying stream, even if there's no data written as part of the flush call itself.
2. Several byte[] array arguments should be byte[] buffer. Technically a breaking change, but the current divergence from the base Stream class is a bug, and bringing them into sync means argument exceptions that emerge from the derived type make sense when used via the base type, as well as then being able to use shared validation logic across all streams (subsequent to these changes).
3. DeflateStream.EndRead/Write needs to do additional state validation.
4. Not a bug, but simplify ReadAsync to match the sync Read implementation flow.
* Add stream conformance tests for Deflate/ZLib/GZip/BrotliStream
* Fix PipeReader/WriterStream argument validation
* Add stream conformance tests for PipeReader/WriterStream
* Remove erroneous asserts from Stream.ReadWriteTask
* Address PR feedback
* Fix a few tests in CI
* [mono] Fix LoadAssemblyRaw to not pin or copy Copying of the assembly and symbols happens later, in `mono_alc_load_raw_bytes`, and the pinning is no longer necessary so long as we have the handle. * Handle null symbol store
* Respect reloadOnChange in UserSecrets in DefaultBuilder
* Make internal API ThrowIfDeserializationInProgress internal also avoid doing costly argument checks * Make methods public again * Update src/libraries/System.Private.CoreLib/src/System/Runtime/Serialization/SerializationInfo.SerializationGuard.cs Co-authored-by: Levi Broderick <[email protected]> Co-authored-by: Levi Broderick <[email protected]>
Co-authored-by: praeclarum <[email protected]>
* Update dependencies from https://github.com/mono/linker build 20201109.1 Microsoft.NET.ILLink.Tasks From Version 6.0.0-alpha.1.20556.1 -> To Version 6.0.0-alpha.1.20559.1 * Update dependencies from https://github.com/mono/linker build 20201109.2 Microsoft.NET.ILLink.Tasks From Version 6.0.0-alpha.1.20556.1 -> To Version 6.0.0-alpha.1.20559.2 * Update dependencies from https://github.com/mono/linker build 20201109.3 Microsoft.NET.ILLink.Tasks From Version 6.0.0-alpha.1.20556.1 -> To Version 6.0.0-alpha.1.20559.3 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
…vert to NI) (#41213) * Convert get_ManagedThreadId(get_CurrentThread) optimization to NamedIntrinsics * drop CORINFO_HELP_INTERNALTHROW and CORINFO_HELP_SEC_UNMGDCODE_EXCPT
…431) For methods which need to show up during stack walks, push/pop an LMF frame and have mono_arch_unwind_frame () handle it. This is needed to be able to support methods which do stack walks like Type.GetType () or Assembly.Load () in the future. Co-authored-by: vargaz <[email protected]>
… every Helix job (#44455)
… (#44345)
* [jit] Emit profiler enter after jit attach; leave before detach
profiler enter code (such as mono_trace_enter_method) must not be
called before a thread attaches to the runtime - otherwise calls like
`mono_domain_get()` will return NULL unexpectedly and then crash.
When detaching, we should call the profiler leave code before the
detach, and suppress it on return.
Simple repro (that relies on pal_signal.c SignalHandlerLoop - which is
a background thread from System.Native that calls back into managed
when there's a SIGCHLD): compile and run this program with `MONO_ENV_OPTIONS=--trace`
```csharp
using System;
using System.Diagnostics;
namespace Repro
{
internal class Program
{
private static void Main(string[] args)
{
using (Process myProcess = new Process())
{
myProcess.StartInfo.UseShellExecute = true;
myProcess.StartInfo.FileName = "echo";
myProcess.StartInfo.Arguments = "hello from shell";
myProcess.StartInfo.CreateNoWindow = true;
myProcess.Start();
myProcess.WaitForExit();
}
Console.ReadKey ();
}
}
}
```
Our scripts normally assume that all test projects are at least two nesting levels deep under $(TestBinDir); the r2rdump test was violating that and, in doing so, it was triggering a failure in CG2 compilation. After I decided to fix this by moving the r2rdump folder under "readytorun" I found out that there already is a r2rdump folder there - apparently Amy during her initial R2RDump implementation authored a basic test suite even though it's disabled right now. I have resolved this by moving the original test under "readytorun/r2rdump" to "readytorun/r2rdump/BasicTests" and I moved Andrew's new test under "readytorun/r2rdump/FrameworkTests". I'm not super happy about the naming but I don't have any better ideas at the moment, I'll be happy to improve this based on PR feedback. Thanks Tomas
…(#44482) This reverts commit a683543.
…sm makefile (#44450) Ensures libSystem.Native.a/libSystem.IO.Compression.Native.a end up in the runtime pack.
Allows nint and nuint to participate in the same optimizations in string building / formatting as do the other primitives.
Add a new inline policy that can be used when a method has profile data. It uses the call site frequency to boost profitability. Size and per-call benefit are currently using the estimates done by the model policy. Not on by default. Set COMPlus_JitInlinePolicyProfile=1 to enable. Add testing to weekly experimental runs.
* Fix MarshalTypedArrayByte and re-enable it. Re-enable TestFunctionApply * Re-enable MarshalTypedArray * Detect when the managed wrapper for a JS object has been collected and create a new one * Remove debugging code * Maintain a small pool of temporary root instances to minimize GC pressure. Improve benchmark * Don't use release_roots in call_method due to varargs overhead * Various call_method optimizations * Checkpoint: Don't rely on finally block for teardown in call path, because it has a performance cost * Checkpoint: Unboxing fast path for primitives * Checkpoint: Fix unboxing fast path * Update bindings to use bound static methods instead of call_method in various places * Address PR feedback * Revert sample and add separate proj for benchmark * Fix benchmark * Revert test change * Fix passing mono object ptrs to bound functions Fix passing strings across the boundary Fix JS strings being truncated at the first null when passed to mono * Implement unboxing for chars Slightly optimize the unboxing slow path * Don't allocate a root buffer for arguments if nothing needs to be rooted. Reuse scratch native buffer across calls unless re-entrant to avoid per-invoke malloc * Fix whitespace damage from merge * Tweaks to try and prevent boxing/gc * Fix typo * Add some tests * Fix test failures * Add more error handling and diagnostic messages Fix a couple broken tests * Repair merge damage * Remove bindings benchmark * Use TypedArray.fill 3-argument version to zero memory * Checkpoint: Introduce format strings * Fix interpolated strings * Test refactoring * Checkpoint: Add more test coverage for bindings and interop * Checkpoint: Enum marshaling works * Improve test coverage * Checkpoint: Unify unboxing of primitive types * Checkpoint: Unify unboxing of primitive types * Checkpoint: Restore fn to satisfy runtime-test.js * Checkpoint: Unify boxing for primitives * Remove now-unused box methods * Don't store names for null method IDs * Fix indentation damage * Add test * Satisfy CI * Accept weaker promises Co-authored-by: Larry Ewing <[email protected]>
…symbol names. (#44235)
* Lazily-allocate StreamWriter's byte[] buffer
The byte[] buffer is used just when flushing to the underlying stream, to store the intermediate bytes as generated from the characters by the encoding. Currently the byte[] buffer is allocated in the constructor, but for relatively small strings written/flushed synchronously (such as those often used with Console.Write), we can avoid the buffer entirely via stack allocation (in Flush); for all other cases, we can allocate it on demand.
(I considered using ArrayPool to rent/return in each Flush{Async} call, but opted not to in this change. Someone could investigate that subsequently.)
* Undo use of static async helpers
In .NET Framework, there was a non-trivial penalty due to StreamWriter deriving from MarshalByRefObject and the impact that had on async methods accessing lifted locals on the state machine (it made them significantly more expensive). The workaround was to make the async methods be statics and pass in all of the relevant instance state needed. That workaround is no longer necessary on core, where MarshalByRefObjects are nops, leaving behind a workaround that is not just clunkier, but actually makes things more expensive because more state needs to be stored on the state machine objects (all of the arguments, which can instead just be accessed off of `this`). Undo that whole change.
* Address PR feedback
…0.1 (#44499) Microsoft.NET.ILLink.Tasks From Version 6.0.0-alpha.1.20559.3 -> To Version 6.0.0-alpha.1.20560.1 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* Specialize `EqualityComparer<string>.Default` Removes ~80 allocations at startup. * Avoid loading Encoding.Unicode just for CodePage * Use fixed instead of GCHandle.Alloc in EventSource.Initialize * Remove lock / lock object from EncodingProvider.AddProvider * Remove lock object from AppDomain.cs * Lazily allocate EventSource's m_createEventLock It's only used on an error path. We don't need to allocate it for each EventSource that's created. * Avoid unnecessary CultureInfo access in derived TextWriters SyncTextWriter already overrides FormatProvider, in which case the t.FormatProvider passed to the base will never be used, so this call is incurring a virtual dispatch for no benefit. And NullTextWriter needn't access InvariantCulture and force it into existence if it isn't yet, as the formatting should never actually be used, and if it is, its FormatProvider override can supply the culture. * Avoid allocating AssemblyLoadContext's dictionary if no direct interaction with ALC AssemblyLoadContext.OnProcessExit gets called by EventSource, which in turn forces s_allContexts into existence in order to lock on it in order to enumerate all active contexts, and if there's been no interaction with AssemblyLoadContext, there won't be any to enumerate. So delay allocate the object. * Address PR feedback * Call EventListener.DisposeOnShutdown from AppContext.OnProcessExit Avoids the need to register with AppContext.ProcessExit, avoiding an EventHandler allocation, and avoids the need in the common case to fire AppContext.ProcessExit, which in turn avoids allocating an AppDomain and EventArgs if they weren't otherwise created, plus it avoids the delegate invocation. * Update src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
Co-authored-by: Geoffrey Kizer <[email protected]>
…ion test fails (#44509) * log exception if Timeout_SetTenMillisecondsOnLoopback_ThrowsWebException test fails * check exception type before checking time range
* Add ICorDebugHeapValue4 * Add EnableGCNotificationEvents deprecation comment
feature/JsonCodeGen
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
75 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR #286 intended to do a merge from runtime-master but we accidentally included a force-push there and then also reapplied some changes from runtime which broke the history between runtime and JsonCodeGen. In order to fix this, we had to revert all changes that went in from that change onwards, and this PR is fixing the original merge and then reapplying the removed changes again. PRs that were removed and are being added back with this are: #286, #319, #328, #330, #332, and #335
cc: @layomia @steveharter