This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
Use Portable PDBs and turn on SourceLink #13964
Closed
Conversation
This file contains 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
|
The corresponding coreFX pull request is dotnet/corefx#24025 |
jkotas
reviewed
Sep 14, 2017
| @@ -72,14 +72,14 @@ | |||
| <DebugSymbols>true</DebugSymbols> | |||
| <Optimize Condition="'$(Optimize)' == '' and '$(Configuration)' == 'Debug'">false</Optimize> | |||
| <Optimize Condition="'$(Optimize)' == '' and '$(Configuration)' == 'Checked'">true</Optimize> | |||
| <DebugType>full</DebugType> | |||
| <DebugType Condition="'$(DebugType)' == ''">full</DebugType> | |||
There was a problem hiding this comment.
I think you can just delete these lines.
There was a problem hiding this comment.
Indeed this was my first version. I put them back because they may be useful if we ever go back to MS-PDB (but that is a pretty low-probability event). I am OK with removing them.
|
|
LEA[b+0] was not eliminated on ARM which cause assertion on code generation
There's no need for that and if the negative array length is not representable in 32 bit we'll end up producing a GT_CNS_INT node that has TYP_INT and a 64 bit value. That's because the original type (always TYP_INT) of the GT_ARR_LENGTH is preserved when changing the node to GT_CNS_INT.
[Arm64] Remove LegacyBackend defines
add repro for DevDiv_495792. fix the issue.
Fix superPMI getHelperFunction
In particular, add "Cross" to the trigger strings.
Add support for priority and update x64 client
Reduce allocations when async methods yield
* extract CheckLclVarSemantics from CheckLIR. * add a test that shows the silent bad execution. * fix the checker. * add the test to the exclude list. * rename consumed to used
Improve thread pool worker thread's spinning for work Closes https://github.com/dotnet/coreclr/issues/5928 Replaced UnfairSemaphore with a new implementation in CLRLifoSemaphore - UnfairSemaphore had a some benefits: - It tracked the number of spinners and avoids waking up waiters as long as the signal count can be satisfied by spinners - Since spinners get priority over waiters, that's the main "unfair" part of it that allows hot threads to remain hot and cold threads to remain cold. However, waiters are still released in FIFO order. - Spinning helps with throughput when incoming work is bursty - All of the above benefits were retained in CLRLifoSemaphore and some were improved: - Similarly to UnfairSemaphore, the number of spinners are tracked and preferenced to avoid waking up waiters - For waiting, on Windows, a I/O completion port is used since it releases waiters in LIFO order. For Unix, added a prioritized wait function to the PAL to register waiters in reverse order for LIFO release behavior. This allows cold waiters to time out more easily since they will be used less frequently. - Similarly to SemaphoreSlim, the number of waiters that were signaled to wake but have not yet woken is tracked to help avoid waking up an excessive number of waiters - Added some YieldProcessorNormalized() calls to the spin loop. This avoids thrashing on Sleep(0) by adding a delay to the spin loop to allow it to be more effective when there are no threads to switch to, or the only other threads to switch to are other similar spinners. - Removed the processor count multiplier on the max spin count and retuned the default max spin count. The processor count multiplier was causing excessive CPU usage on machines with many processors.
* Fix a crash that occurs when a provider is registered after the configuration object has been destroyed. * Code review feedback.
Fix ARM64 trigger strings to match job names
Add Span-based Convert Base64 methods
[Arm64] Implement GT_XADD, GT_XCHG, GT_CMPXCHG ...
Check for native value type has to be first to handle byref-like native value types correctly.
* impImportCall: make the failed imports obvious. Delete unnecessary and confusing dependency on callRetType. * delete unreached code that calls impImportCall
…net#14341) * undef NULL to fix compilation problems on FreeBSD after 08d39dd. * remove unneccesary #ifndef NULL
…Dependencies Update BuildTools, CoreClr, CoreFx, PgoData to prerelease-02106-01, preview1-25806-01, preview1-25806-02, master-20171006-0051, respectively (master)
Fix #14199: propagate `GTF_EXCEPT` bits from the end of `GT_FIELD_LIST` lists to the beginning, to avoid "Missing flags on tree" asserts. Fix #14198: for RyuJIT/arm32, `GT_BITCAST` needs to be a MultiRegOp. This is required when a varargs function, which includes the tailcall helper, needs to pass a double in integer registers. We can end up with `GT_PUTARG_REG/long(GT_BITCAST/long(double tree))`. Fixed various GenTree node flags dumping issues.
Add Span-based methods to DateTime{Offset}
My recent delegate optimization for async methods introduced a race condition that breaks posting back to a captured context. If the awaited task completes between the time we check IsCompleted and when we try to store the continuation into the Task, we need to queue the continuation for execution, but my change incorrectly queued it to the thread pool rather than to the captured context.
Fix handling of continuations for captured context
Delete some dead code related to Windows Phone and code access security
Fix RyuJIT/arm32 asserts
This change fixes a break in build of coreclr on machines with libnuma installed. The problem was that the numa header contains a couple of inlined functions and we were using one of them. That made it to have a hard reference to a function from the numa library that we need to be soft so that coreclr can work even on machines without the libnuma installed.
* GT_DIV_HI and GT_MOD_HI are not used anywhere * genCodeForBinary doesn't handle GT_MUL_LONG * OperIsHigh is not used anywhere
* [GDBJIT] Fix DW_AT_comp_dir setting We should use cuPath to set dirPath in NotifyGdb::EmitDebugInfo instead of DebugString[1]. * [GDBJIT] Make gdbjit thread-safe NotifyGdb::MethodPrepared method can be called from multiple threads simultaneously in this case gdbjit will work incorrectly as it uses global variable without synchronization.
Fix incorrect name of class in RyuJIT overview
* Fix zap and ready to run disabling While the COMPlus_ZapDisable and COMPlus_ReadyToRun config settings can be used to disable using crossgened / ready to run images, loading IL from such images fails. This change foxes that. * Reflect PR feedback Remove ReadyToRunInfo::IsReadyToRunEnabled from Module::Initialize again and move the same check in the ReadyToRunInfo::Initialize before we start checking the PE file properties.
Removed unused opers and code
This makes us uniform. Allows VSCode (which does not suport MSPdbs) use on Windows.
This makes us uniform. Allows VSCode (which does not suport MSPdbs) use on Windows.
|
Turning on source link support in #14399, |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This pull request, as well as its companion in the CoreFX repository are tiny (they basically set an MSBUILD variable), but the result is that we generate Portable PDBs rather than MS-PDBs during the build).
The immediate value of this change is that we can turn on SourceLink support which allows Visual Studio to 'just work' and step into source it fetches from this Github repository. This is an important step in getting this working for our shipped builds.
It is possible to get SourceLink working without generating Portable PDBs but I believe we want to move in this direction sooner rather than later because.
In short, uniformity is good, and this makes us significantly more uniform. The only issue is what 'breaks'. Because of (4), it is not clear that any important scenario is actually broken by the change. To the degree that we publish PDBs via the MS symbol server, those were converted before publication so things will work as the do now. For private builds, VS works with portable PDBs. Tests should be using portable (so they can run on Linux too). Now I am sure that we might break something, but we are likely to be close enough that it is easier/better to simply try it and clean it up (or revert it), if we run into issues.
But I have marked this NO-MERGE for the time being so that we can vet any concerns. If after several days of vetting we have addressed all concerns we would be in a position to proceed.
Please include anyone you believe may be impacted by this.
@dagood @mikem8361 @ericstj @jkotas @lt72 @Petermarcu @markwilkie @weshaggard @tmat @Eilon @karelz @danmosemsft