The Wayback Machine - https://web.archive.org/web/20210826170717/https://github.com/dotnet/runtime/pull/43365
Skip to content

/ runtime

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

Jump to bottom

Replace ReusableTextReader with shared ArrayPool #43365

Merged
merged 1 commit into from Oct 14, 2020
Merged

Replace ReusableTextReader with shared ArrayPool #43365

merged 1 commit into from Oct 14, 2020

Conversation

@am11
Copy link
Contributor

@am11 am11 commented Oct 13, 2020

ReusableTextReader is only used internally on Linux and it allocates buffers in the ctor. Instantiating it in a static context makes the CoreLib linker unfriendly. This change replaces the usage of RTR with ArrayPool<T>, and deletes the class.

Fixes #43337
@am11
Copy link
Contributor Author

@am11 am11 commented Oct 13, 2020

cc @jkotas, @danmosemsft
@stephentoub
stephentoub reviewed Oct 13, 2020
View changes
.../Common/src/Interop/Linux/procfs/Interop.ProcFsStat.TryReadStatusFile.cs Outdated Show resolved Hide resolved
@stephentoub
stephentoub reviewed Oct 13, 2020
View changes
.../Common/src/Interop/Linux/procfs/Interop.ProcFsStat.TryReadStatusFile.cs Outdated Show resolved Hide resolved
@stephentoub
stephentoub reviewed Oct 13, 2020
View changes
.../Common/src/Interop/Linux/procfs/Interop.ProcFsStat.TryReadStatusFile.cs Outdated Show resolved Hide resolved
@jkotas
jkotas reviewed Oct 13, 2020
View changes
.../Common/src/Interop/Linux/procfs/Interop.ProcFsStat.TryReadStatusFile.cs Show resolved Hide resolved
@jkotas
jkotas reviewed Oct 13, 2020
View changes
.../Common/src/Interop/Linux/procfs/Interop.ProcFsStat.TryReadStatusFile.cs Outdated Show resolved Hide resolved
@jkotas
jkotas reviewed Oct 13, 2020
View changes
.../Common/src/Interop/Linux/procfs/Interop.ProcFsStat.TryReadStatusFile.cs Outdated Show resolved Hide resolved
@jkotas
jkotas reviewed Oct 13, 2020
View changes
.../Common/src/Interop/Linux/procfs/Interop.ProcFsStat.TryReadStatusFile.cs Outdated Show resolved Hide resolved
@jkotas
jkotas reviewed Oct 13, 2020
View changes
.../Common/src/Interop/Linux/procfs/Interop.ProcFsStat.TryReadStatusFile.cs Outdated Show resolved Hide resolved
@jkotas
jkotas reviewed Oct 13, 2020
View changes
.../Common/src/Interop/Linux/procfs/Interop.ProcFsStat.TryReadStatusFile.cs Outdated Show resolved Hide resolved
@jkotas
jkotas approved these changes Oct 13, 2020
View changes
@jkotas
jkotas reviewed Oct 13, 2020
View changes
.../Common/src/Interop/Linux/procfs/Interop.ProcFsStat.TryReadStatusFile.cs Outdated Show resolved Hide resolved
@am11
Replace ReusableTextReader with shared ArrayPool
a0e3b49
@danmoseley
danmoseley reviewed Oct 14, 2020
View changes
src/libraries/Common/src/Interop/Linux/procfs/Interop.ProcFsStat.TryReadStatusFile.cs
return false;
}
finally
{
ArrayPool<byte>.Shared.Return(bytes);

This comment has been minimized.

Sign in to view
@danmoseley

danmoseley Oct 14, 2020
Member

@jkotas I have seen in some cases we don't bother to return in exceptional cases. Yet, I could imagine for some OS reason, we are always failing to read this file, and we are called often, so we drain the array pool. I guess it depends on the context?

This comment has been minimized.

Sign in to view
@jkotas

jkotas Oct 14, 2020
Member

Yep, it depends on the context. I think the try/finally is appropriate here.
@danmoseley
danmoseley approved these changes Oct 14, 2020
View changes
@danmoseley
Copy link
Member

@danmoseley danmoseley commented Oct 14, 2020

failure is #43200
@danmoseley danmoseley merged commit 041826b into dotnet:master Oct 14, 2020
98 of 100 checks passed
98 of 100 checks passed
@azure-pipelines
runtime Build #20201013.82 had test failures
Details
@azure-pipelines
runtime (Build Browser wasm Release AllSubsets_Mono_RuntimeTests) Build Browser wasm Release AllSubsets_Mono_RuntimeTests failed
Details
license/cla All CLA requirements met.
Details
@azure-pipelines
runtime (Build Android arm Release AllSubsets_Mono) Build Android arm Release AllSubsets_Mono succeeded
Details
@azure-pipelines
runtime (Build Android arm64 Debug AllSubsets_Mono) Build Android arm64 Debug AllSubsets_Mono succeeded
Details
@azure-pipelines
runtime (Build Android x64 Release AllSubsets_Mono) Build Android x64 Release AllSubsets_Mono succeeded
Details
@azure-pipelines
runtime (Build Android x86 Debug AllSubsets_Mono) Build Android x86 Debug AllSubsets_Mono succeeded
Details
@azure-pipelines
runtime (Build Browser wasm Release AllSubsets_Mono) Build Browser wasm Release AllSubsets_Mono succeeded
Details
@azure-pipelines
runtime (Build Linux arm Debug AllSubsets_Mono) Build Linux arm Debug AllSubsets_Mono succeeded
Details
@azure-pipelines
runtime (Build Linux arm64 Debug AllSubsets_Mono_LLVMJIT) Build Linux arm64 Debug AllSubsets_Mono_LLVMJIT succeeded
Details
@azure-pipelines
runtime (Build Linux arm64 Release AllSubsets_Mono_LLVMAOT) Build Linux arm64 Release AllSubsets_Mono_LLVMAOT succeeded
Details
@azure-pipelines
runtime (Build Linux x64 Debug AllSubsets_Mono_LLVMJIT) Build Linux x64 Debug AllSubsets_Mono_LLVMJIT succeeded
Details
@azure-pipelines
runtime (Build Linux x64 Release AllSubsets_Mono_LLVMAOT) Build Linux x64 Release AllSubsets_Mono_LLVMAOT succeeded
Details
@azure-pipelines
runtime (Build Linux_musl x64 Release AllSubsets_Mono) Build Linux_musl x64 Release AllSubsets_Mono succeeded
Details
@azure-pipelines
runtime (Build OSX x64 Debug AllSubsets_Mono_LLVMAOT) Build OSX x64 Debug AllSubsets_Mono_LLVMAOT succeeded
Details
@azure-pipelines
runtime (Build OSX x64 Release AllSubsets_Mono_LLVMJIT) Build OSX x64 Release AllSubsets_Mono_LLVMJIT succeeded
Details
@azure-pipelines
runtime (Build iOS arm Release AllSubsets_Mono) Build iOS arm Release AllSubsets_Mono succeeded
Details
@azure-pipelines
runtime (Build iOS arm64 Debug AllSubsets_Mono) Build iOS arm64 Debug AllSubsets_Mono succeeded
Details
@azure-pipelines
runtime (Build iOS x64 Release AllSubsets_Mono) Build iOS x64 Release AllSubsets_Mono succeeded
Details
@azure-pipelines
runtime (Build iOS x86 Debug AllSubsets_Mono) Build iOS x86 Debug AllSubsets_Mono succeeded
Details
@azure-pipelines
runtime (Build tvOS arm64 Release AllSubsets_Mono) Build tvOS arm64 Release AllSubsets_Mono succeeded
Details
@azure-pipelines
runtime (Build tvOS x64 Debug AllSubsets_Mono) Build tvOS x64 Debug AllSubsets_Mono succeeded
Details
@azure-pipelines
runtime (Checkout) Checkout succeeded
Details
@azure-pipelines
runtime (CoreCLR Pri0 Runtime Tests Run Linux arm checked) CoreCLR Pri0 Runtime Tests Run Linux arm checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Pri0 Runtime Tests Run Linux arm64 checked) CoreCLR Pri0 Runtime Tests Run Linux arm64 checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Pri0 Runtime Tests Run Linux x64 checked) CoreCLR Pri0 Runtime Tests Run Linux x64 checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Pri0 Runtime Tests Run OSX x64 checked) CoreCLR Pri0 Runtime Tests Run OSX x64 checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Pri0 Runtime Tests Run Windows_NT arm64 checked) CoreCLR Pri0 Runtime Tests Run Windows_NT arm64 checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Pri0 Runtime Tests Run Windows_NT x64 checked) CoreCLR Pri0 Runtime Tests Run Windows_NT x64 checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Pri0 Runtime Tests Run Windows_NT x86 checked) CoreCLR Pri0 Runtime Tests Run Windows_NT x86 checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build FreeBSD x64 release) CoreCLR Product Build FreeBSD x64 release succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Linux arm checked) CoreCLR Product Build Linux arm checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Linux arm release) CoreCLR Product Build Linux arm release succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Linux arm64 checked) CoreCLR Product Build Linux arm64 checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Linux arm64 release) CoreCLR Product Build Linux arm64 release succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Linux x64 checked) CoreCLR Product Build Linux x64 checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Linux x64 release) CoreCLR Product Build Linux x64 release succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Linux_musl arm checked) CoreCLR Product Build Linux_musl arm checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Linux_musl arm release) CoreCLR Product Build Linux_musl arm release succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Linux_musl arm64 checked) CoreCLR Product Build Linux_musl arm64 checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Linux_musl arm64 release) CoreCLR Product Build Linux_musl arm64 release succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Linux_musl x64 checked) CoreCLR Product Build Linux_musl x64 checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Linux_musl x64 release) CoreCLR Product Build Linux_musl x64 release succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build OSX arm64 checked) CoreCLR Product Build OSX arm64 checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build OSX arm64 release) CoreCLR Product Build OSX arm64 release succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build OSX x64 checked) CoreCLR Product Build OSX x64 checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build OSX x64 release) CoreCLR Product Build OSX x64 release succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Windows_NT arm checked) CoreCLR Product Build Windows_NT arm checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Windows_NT arm release) CoreCLR Product Build Windows_NT arm release succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Windows_NT arm64 checked) CoreCLR Product Build Windows_NT arm64 checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Windows_NT arm64 release) CoreCLR Product Build Windows_NT arm64 release succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Windows_NT x64 checked) CoreCLR Product Build Windows_NT x64 checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Windows_NT x64 release) CoreCLR Product Build Windows_NT x64 release succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Windows_NT x86 checked) CoreCLR Product Build Windows_NT x86 checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Product Build Windows_NT x86 release) CoreCLR Product Build Windows_NT x86 release succeeded
Details
@azure-pipelines
runtime (CoreCLR Tools Unit Tests Linux x64 checked) CoreCLR Tools Unit Tests Linux x64 checked succeeded
Details
@azure-pipelines
runtime (CoreCLR Common Pri0 Test Build AnyOS AnyCPU checked) CoreCLR Common Pri0 Test Build AnyOS AnyCPU checked succeeded
Details
@azure-pipelines
runtime (CoreCLR GCC Product Build Linux x64 checked) CoreCLR GCC Product Build Linux x64 checked succeeded
Details
@azure-pipelines
runtime (Formatting Linux x64) Formatting Linux x64 succeeded
Details
@azure-pipelines
runtime (Installer Build and Test coreclr FreeBSD_x64 Release) Installer Build and Test coreclr FreeBSD_x64 Release succeeded
Details
@azure-pipelines
runtime (Installer Build and Test coreclr Linux_arm Debug) Installer Build and Test coreclr Linux_arm Debug succeeded
Details
@azure-pipelines
runtime (Installer Build and Test coreclr Linux_arm64 Release) Installer Build and Test coreclr Linux_arm64 Release succeeded
Details
@azure-pipelines
runtime (Installer Build and Test coreclr Linux_musl_arm Debug) Installer Build and Test coreclr Linux_musl_arm Debug succeeded
Details
@azure-pipelines
runtime (Installer Build and Test coreclr Linux_musl_arm64 Debug) Installer Build and Test coreclr Linux_musl_arm64 Debug succeeded
Details
@azure-pipelines
runtime (Installer Build and Test coreclr Linux_musl_x64 Release) Installer Build and Test coreclr Linux_musl_x64 Release succeeded
Details
@azure-pipelines
runtime (Installer Build and Test coreclr Linux_x64 Release) Installer Build and Test coreclr Linux_x64 Release succeeded
Details
@azure-pipelines
runtime (Installer Build and Test coreclr OSX_arm64 Release) Installer Build and Test coreclr OSX_arm64 Release succeeded
Details
@azure-pipelines
runtime (Installer Build and Test coreclr OSX_x64 Release) Installer Build and Test coreclr OSX_x64 Release succeeded
Details
@azure-pipelines
runtime (Installer Build and Test coreclr Windows_NT_arm Debug) Installer Build and Test coreclr Windows_NT_arm Debug succeeded
Details
@azure-pipelines
runtime (Installer Build and Test coreclr Windows_NT_arm64 Debug) Installer Build and Test coreclr Windows_NT_arm64 Debug succeeded
Details
@azure-pipelines
runtime (Installer Build and Test coreclr Windows_NT_x64 Release) Installer Build and Test coreclr Windows_NT_x64 Release succeeded
Details
@azure-pipelines
runtime (Installer Build and Test coreclr Windows_NT_x86 Debug) Installer Build and Test coreclr Windows_NT_x86 Debug succeeded
Details
@azure-pipelines
runtime (Libraries Build FreeBSD x64 Debug) Libraries Build FreeBSD x64 Debug succeeded
Details
@azure-pipelines
runtime (Libraries Build Linux arm Release) Libraries Build Linux arm Release succeeded
Details
@azure-pipelines
runtime (Libraries Build Linux arm64 Debug) Libraries Build Linux arm64 Debug succeeded
Details
@azure-pipelines
runtime (Libraries Build Linux x64 Debug) Libraries Build Linux x64 Debug succeeded
Details
@azure-pipelines
runtime (Libraries Build Linux_musl arm Release) Libraries Build Linux_musl arm Release succeeded
Details
@azure-pipelines
runtime (Libraries Build Linux_musl arm64 Release) Libraries Build Linux_musl arm64 Release succeeded
Details
@azure-pipelines
runtime (Libraries Build Linux_musl x64 Debug) Libraries Build Linux_musl x64 Debug succeeded
Details
@azure-pipelines
runtime (Libraries Build OSX arm64 Debug) Libraries Build OSX arm64 Debug succeeded
Details
@azure-pipelines
runtime (Libraries Build OSX x64 Debug) Libraries Build OSX x64 Debug succeeded
Details
@azure-pipelines
runtime (Libraries Build Windows_NT allConfigurations x64 Debug) Libraries Build Windows_NT allConfigurations x64 Debug succeeded
Details
@azure-pipelines
runtime (Libraries Build Windows_NT arm Release) Libraries Build Windows_NT arm Release succeeded
Details
@azure-pipelines
runtime (Libraries Build Windows_NT arm64 Release) Libraries Build Windows_NT arm64 Release succeeded
Details
@azure-pipelines
runtime (Libraries Build Windows_NT net48 x86 Release) Libraries Build Windows_NT net48 x86 Release succeeded
Details
@azure-pipelines
runtime (Libraries Build Windows_NT x64 Debug) Libraries Build Windows_NT x64 Debug succeeded
Details
@azure-pipelines
runtime (Libraries Build Windows_NT x86 Debug) Libraries Build Windows_NT x86 Debug succeeded
Details
@azure-pipelines
runtime (Libraries Build Windows_NT x86 Release) Libraries Build Windows_NT x86 Release succeeded
Details
@azure-pipelines
runtime (Libraries Test Run checked coreclr Linux arm Release) Libraries Test Run checked coreclr Linux arm Release succeeded
Details
@azure-pipelines
runtime (Libraries Test Run checked coreclr Linux x64 Debug) Libraries Test Run checked coreclr Linux x64 Debug succeeded
Details
@azure-pipelines
runtime (Libraries Test Run checked coreclr Linux_musl arm Release) Libraries Test Run checked coreclr Linux_musl arm Release succeeded
Details
@azure-pipelines
runtime (Libraries Test Run checked coreclr Linux_musl arm64 Release) Libraries Test Run checked coreclr Linux_musl arm64 Release succeeded
Details
@azure-pipelines
runtime (Libraries Test Run checked coreclr Linux_musl x64 Debug) Libraries Test Run checked coreclr Linux_musl x64 Debug succeeded
Details
@azure-pipelines
runtime (Libraries Test Run checked coreclr OSX x64 Debug) Libraries Test Run checked coreclr OSX x64 Debug succeeded
Details
@azure-pipelines
runtime (Libraries Test Run checked coreclr Windows_NT x64 Debug) Libraries Test Run checked coreclr Windows_NT x64 Debug succeeded
Details
@azure-pipelines
runtime (Libraries Test Run checked coreclr Windows_NT x86 Release) Libraries Test Run checked coreclr Windows_NT x86 Release succeeded
Details
@azure-pipelines
runtime (Libraries Test Run release coreclr Linux x64 Debug) Libraries Test Run release coreclr Linux x64 Debug succeeded
Details
@azure-pipelines
runtime (Libraries Test Run release coreclr Linux_musl x64 Debug) Libraries Test Run release coreclr Linux_musl x64 Debug succeeded
Details
@azure-pipelines
runtime (Libraries Test Run release coreclr OSX x64 Debug) Libraries Test Run release coreclr OSX x64 Debug succeeded
Details
@azure-pipelines
runtime (Libraries Test Run release coreclr Windows_NT x64 Debug) Libraries Test Run release coreclr Windows_NT x64 Debug succeeded
Details
@danmoseley
Copy link
Member

@danmoseley danmoseley commented Oct 14, 2020

thanks @am11
@dotnet dotnet deleted a comment from jkotas Oct 14, 2020
@am11 am11 deleted the am11:feature/cleanup/purge-reusabletextreader branch Oct 14, 2020
@runfoapp runfoapp bot mentioned this pull request Oct 14, 2020
Closed
@msftbot msftbot bot locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime
4 participants
@am11 @danmoseley @stephentoub @jkotas