-
Notifications
You must be signed in to change notification settings - Fork 199
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
[NativeAOT-LLVM] support System.Net.Http.HttpClient
on WASIp2
#2614
Changes from 5 commits
beabe5a
28d758f
e32a223
e79642e
2f3ce60
18f9f6c
ebdbabf
9d9ed59
4879365
137614e
b25bc9e
5f35285
ddbd68a
e989a63
98bf4dc
757eeb9
dfdd8bc
1736bdd
dd5f8ae
68ec226
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
$RootPath = $Args[0] | ||
|
||
$NpmExePath = $Env:NPM_EXECUTABLE | ||
|
||
Set-Location -Path $RootPath | ||
|
||
& $NpmExePath install @bytecodealliance/jco | ||
& $NpmExePath install @bytecodealliance/preview2-shim | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -462,7 +462,7 @@ | |||
</ItemGroup> | ||||
|
||||
<!-- TODO-LLVM: This is not upstreamable because it makes the build runtime-specific. --> | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There is another one above (line 27) that needs to be deleted. |
||||
<ItemGroup Condition="'$(TargetPlatformIdentifier)' == 'wasi' and '$(RuntimeFlavor)' != 'CoreCLR'"> | ||||
<ItemGroup Condition="'$(TargetPlatformIdentifier)' == 'wasi'"> | ||||
<Reference Include="System.Threading.Thread" /> | ||||
<Compile Include="System\Net\Http\WasiHttpHandler\WasiHttpHandler.cs" /> | ||||
<Compile Include="System\Net\Http\WasiHttpHandler\WasiHttp.cs" /> | ||||
|
@@ -532,5 +532,4 @@ | |||
<ItemGroup> | ||||
<None Include="Resources\SR.resx" /> | ||||
</ItemGroup> | ||||
dicej marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
|
||||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,7 @@ | |
using System.Threading.Tasks; | ||
using System.Diagnostics.Metrics; | ||
// TODO-LLVM: This is not upstreamable and should be deleted when https://github.com/dotnet/runtimelab/pull/2614 is merged | ||
#if TARGET_WASI && !NATIVE_AOT | ||
#if TARGET_WASI | ||
Comment on lines
13
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I presume the TODOs here should be removed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm rewriting the whole file upstream to respect child resources and more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wasn't sure if the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There were introduced very recently in https://github.com/dotnet/runtimelab/pull/2605/files. I assumed it was a fix for some build break. |
||
using System.Diagnostics; | ||
using System.Net.Http.Metrics; | ||
using HttpHandlerType = System.Net.Http.WasiHttpHandler; | ||
|
@@ -30,7 +30,7 @@ public partial class HttpClientHandler : HttpMessageHandler | |
private readonly HttpHandlerType _underlyingHandler; | ||
|
||
// TODO-LLVM: This is not upstreamable and !NATIVE_AOT should be reverted when https://github.com/dotnet/runtimelab/pull/2614 is merged | ||
#if TARGET_BROWSER || (TARGET_WASI && !NATIVE_AOT) | ||
#if TARGET_BROWSER || TARGET_WASI | ||
private IMeterFactory? _meterFactory; | ||
private HttpMessageHandler? _firstHandler; // DiagnosticsHandler or MetricsHandler, depending on global configuration. | ||
|
||
|
@@ -101,7 +101,7 @@ protected override void Dispose(bool disposing) | |
public IMeterFactory? MeterFactory | ||
{ | ||
// TODO-LLVM: This is not upstreamable and !NATIVE_AOT should be reverted when https://github.com/dotnet/runtimelab/pull/2614 is merged | ||
#if TARGET_BROWSER || (TARGET_WASI && !NATIVE_AOT) | ||
#if TARGET_BROWSER || TARGET_WASI | ||
get => _meterFactory; | ||
set | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,9 +13,9 @@ public sealed partial class Thread | |
{ | ||
// these methods are temporarily accessed via UnsafeAccessor from generated code until we have it in public API, probably in WASI preview3 and promises | ||
#if TARGET_WASI | ||
internal static System.Threading.Tasks.Task RegisterWasiPollableHandle(int handle) | ||
internal static System.Threading.Tasks.Task RegisterWasiPollableHandle(int handle, CancellationToken cancellationToken) | ||
{ | ||
return WasiEventLoop.RegisterWasiPollableHandle(handle); | ||
return WasiEventLoop.RegisterWasiPollableHandle(handle, cancellationToken); | ||
} | ||
|
||
internal static int PollWasiEventLoopUntilResolved(Task<int> mainTask) | ||
|
@@ -33,6 +33,10 @@ internal static int PollWasiEventLoopUntilResolved(Task<int> mainTask) | |
return mainTask.Result; | ||
} | ||
|
||
internal static void DispatchWasiEventLoop() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should not be necessary anymore There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was planning to ask you about this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good question. I was also thinking about it but then I forgot about it. Yes, I think we could add another PollWasiEventLoopUntilResolved with generic signature. |
||
{ | ||
WasiEventLoop.DispatchWasiEventLoop(); | ||
} | ||
#endif | ||
|
||
// the closest analog to Sleep(0) on Unix is sched_yield | ||
|
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.
Most library changes here should be upstreamed (we will merge them here before that, but it should only be temporary).
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 have many such changes in messy PR here dotnet/runtime#105838
I don't expect to merge it in current form.
The problem is that attributes are visible on the runtime API, so we better do it right on the first attempt.
But I don't know exactly what would be PNSE and what would eventually work. And that will change with next WASI preview.
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'll admit I've been using this PR as a dumping ground for miscellaneous WASI fixes as I build demo apps (this one came up when testing AspNetCore). Happy to open separate, upstream PRs as appropriate, but I figured I'd collect them all here first, at least temporarily.
@pavelsavara: yeah, I've been avoiding attributes for the time being since I also don't know which features might be supported in the future. I'm pretty sure WASI will never support signal handling, but I could imagine it might support file notification someday.