Skip to content

Use NativeMemory in System.Data.Odbc - #85966

Closed
xtqqczze wants to merge 4 commits into
dotnet:mainfrom
xtqqczze:NativeMemory.Odbc
Closed

Use NativeMemory in System.Data.Odbc#85966
xtqqczze wants to merge 4 commits into
dotnet:mainfrom
xtqqczze:NativeMemory.Odbc

Conversation

@xtqqczze

@xtqqczzextqqczze commented May 9, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

Contributes to #54297.

Although target frameworks for System.Data.Odbc include netstandard2.0 and $(NetFrameworkMinimum), these platforms are not supported (see #78550), therefore we can use the .NET 6 NativeMemory APIs.

@ghostghost added community-contribution Indicates that the PR has been added by a community member area-System.Data labels May 9, 2023
@ghost

ghost commented May 9, 2023

Copy link
Copy Markdown

Tagging subscribers to this area: @roji, @ajcvickers
See info in area-owners.md if you want to be subscribed.

Issue Details

Contributes to #54297.

Author:xtqqczze
Assignees:-
Labels:

area-System.Data, community-contribution

Milestone:-

@xtqqczze

xtqqczze commented May 9, 2023•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Related: #54468, cc: @reflectronic.

@xtqqczze
xtqqczzeforce-pushed the NativeMemory.Odbc branch from aae5920 to a99952dCompareMay 9, 2023 09:43
@build-analysisbuild-analysisBot mentioned this pull request May 9, 2023
Closed
@Wraith2

Wraith2 commented May 9, 2023

Copy link
Copy Markdown
Contributor

Does this have any specific benefits?

@xtqqczze

xtqqczze commented May 9, 2023

Copy link
Copy Markdown
ContributorAuthor

Does this have any specific benefits?

Use of LocalAlloc is deprecated and NativeMemory.Alloc is faster.

EgorBo
EgorBo reviewed May 9, 2023
View reviewed changes
Comment threadsrc/libraries/System.Data.Odbc/src/Common/System/Data/Common/SafeNativeMethods.cs Outdated
var zeroes = new byte[length];
Marshal.Copy(zeroes, 0, ptr, length);
#if !NET7_0_OR_GREATER
new Span<byte>((void*)ptr, length).Clear();

@EgorBoEgorBoMay 9, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just Span.Clear for all TFMs ?

@xtqqczzextqqczzeMay 9, 2023•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is marginally better, see https://www.diffchecker.com/Gxzd78oe

More significantly - the #if indicates we could just use NativeMemory.Clear once .NET 6 is end-of-life.

@EgorBoEgorBoMay 9, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is marginally better, see https://www.diffchecker.com/Gxzd78oe

It seems that you compared codegen for constant length which is not the case here. Overall, Span.Clear should be better e.g. it's faster for small sizes as it won't pay price for the interop machinery so I assume it's better to just always use that instead of #if-else. If it's for some reason slower it can be a good motivation for us to fix that in BCL/JIT

@xtqqczzextqqczzeMay 9, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is not for constant length, see https://csharp.godbolt.org/z/a157vhTT9

@xtqqczzextqqczzeMay 9, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation for NativeMemory.Clear actually uses SpanHelpers.ClearWithoutReferences(ref *(byte*)ptr, byteCount)

@xtqqczzextqqczzeMay 9, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, since NativeMemory.Alloc guarantees ptr is pointer-aligned, and Length is always IntPtr aligned, we can use Unsafe.InitBlock.

@runfoapprunfoappBot mentioned this pull request May 9, 2023
Closed
@xtqqczze

xtqqczze commented May 9, 2023

Copy link
Copy Markdown
ContributorAuthor

Test failures are unrelated and have been linked to existing issues.

@xtqqczze
xtqqczzeforce-pushed the NativeMemory.Odbc branch from bf12034 to b593c9bCompareMay 9, 2023 21:42
@build-analysisbuild-analysisBot mentioned this pull request May 10, 2023
Open
stephentoub
stephentoub reviewed May 15, 2023
View reviewed changes
Comment threadsrc/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
protected DbBuffer(int initialSize) : base(IntPtr.Zero, true)
protected unsafe DbBuffer(int initialSize) : base(IntPtr.Zero, true)
{
Debug.Assert(initialSize % IntPtr.Size == 0, $"Expected aligned {nameof(initialSize)}.");

@stephentoubstephentoubMay 15, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What guarantees this?

@xtqqczzextqqczzeMay 25, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DbBuffer is is only instantiated from the derived class CNativeBuffer.

The buffer size is either 4096 bytes, or a larger value calculated in CalcParameterBufferSize.

runtime/src/libraries/System.Data.Odbc/src/System/Data/Odbc/OdbcParameterCollection.cs

Lines 112 to 128 in b593c9b

internalintCalcParameterBufferSize(OdbcCommandcommand)
{
// Calculate the size of the buffer we need
intparameterBufferSize=0;
for(inti=0;i<Count;++i)
{
if(_rebindCollection)
{
this[i].HasChanged=true;
}
this[i].PrepareForBind(command,(short)(i+1),refparameterBufferSize);
parameterBufferSize=(parameterBufferSize+(IntPtr.Size-1))&~(IntPtr.Size-1);// align buffer;
}
returnparameterBufferSize;
}

@xtqqczzextqqczzeMay 25, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it makes sense to calculate aligned buffer size in the ctor instead.

rolfbjarne added a commit to rolfbjarne/macios that referenced this pull request Jun 2, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
1efc89e
Ref: dotnet/runtime#85996
rolfbjarne added a commit to rolfbjarne/macios that referenced this pull request Jun 6, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
19a2c9f
Ref: dotnet/runtime#85996
rolfbjarne added a commit to rolfbjarne/macios that referenced this pull request Jun 6, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
2843098
Ref: dotnet/runtime#85996
rolfbjarne added a commit to rolfbjarne/macios that referenced this pull request Jun 7, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
b44c546
Ref: dotnet/runtime#85996
rolfbjarne added a commit to rolfbjarne/macios that referenced this pull request Jun 9, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
1b3a525
Ref: dotnet/runtime#85996
rolfbjarne added a commit to rolfbjarne/macios that referenced this pull request Jun 13, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
323d2ea
Ref: dotnet/runtime#85996
rolfbjarne added a commit to rolfbjarne/macios that referenced this pull request Jun 15, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
76495ad
Ref: dotnet/runtime#85996
@xtqqczze
xtqqczze marked this pull request as draft September 11, 2023 14:52
@build-analysisbuild-analysisBot mentioned this pull request Sep 11, 2023
Closed
@ghostghost closed this Oct 11, 2023
@ghost

ghost commented Oct 11, 2023

Copy link
Copy Markdown

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@ghostghost locked as resolved and limited conversation to collaborators Nov 10, 2023
This pull request was closed.
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@EgorBoEgorBoEgorBo left review comments
@stephentoubstephentoubstephentoub left review comments

Assignees

No one assigned

Labels

area-System.Datacommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@xtqqczze@Wraith2@EgorBo@stephentoub
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Use `NativeMemory` in `System.Data.Odbc` by xtqqczze · Pull Request #85966 · dotnet/runtime · GitHub
Skip to content
Use NativeMemory in System.Data.Odbc - #85966
Closed
xtqqczze wants to merge 4 commits into
dotnet:mainfrom 
xtqqczze:NativeMemory.Odbc
Closed
Use NativeMemory in System.Data.Odbc#85966
xtqqczze wants to merge 4 commits into
dotnet:mainfrom 
xtqqczze:NativeMemory.Odbc
Conversation
@xtqqczze
@xtqqczzextqqczze
commented
May 9, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Contributes to #54297.
Although target frameworks for System.Data.Odbc include netstandard2.0 and $(NetFrameworkMinimum), these platforms are not supported (see #78550), therefore we can use the .NET 6 NativeMemory APIs.
@ghostghost
added

community-contribution
Indicates that the PR has been added by a community member
area-System.Data

labels
May 9, 2023
@ghost
ghost
commented
May 9, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @roji, @ajcvickers

See info in area-owners.md if you want to be subscribed.
Issue Details
Contributes to #54297.
Author:xtqqczze
Assignees:-
Labels:
area-System.Data, community-contribution
Milestone:-
@xtqqczze
xtqqczze
commented
May 9, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Related: #54468, cc: @reflectronic.
@xtqqczze
xtqqczzeforce-pushed
the
NativeMemory.Odbc
branch
from
aae5920 to
a99952dCompareMay 9, 2023 09:43
@build-analysisbuild-analysisBot
mentioned this pull request
May 9, 2023

Closed

@Wraith2
Wraith2
commented
May 9, 2023

Copy link


Copy Markdown

Contributor
Does this have any specific benefits?
@xtqqczze
xtqqczze
commented
May 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Does this have any specific benefits?
Use of LocalAlloc is deprecated and NativeMemory.Alloc is faster.
EgorBo
EgorBo
reviewed
May 9, 2023
View reviewed changes
Comment threadsrc/libraries/System.Data.Odbc/src/Common/System/Data/Common/SafeNativeMethods.cs
Outdated

 var zeroes = new byte[length];
 Marshal.Copy(zeroes, 0, ptr, length);
#if !NET7_0_OR_GREATER
 new Span<byte>((void*)ptr, length).Clear();
@EgorBoEgorBoMay 9, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just Span.Clear for all TFMs ?
@xtqqczzextqqczzeMay 9, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is marginally better, see https://www.diffchecker.com/Gxzd78oe
More significantly - the #if indicates we could just use NativeMemory.Clear once .NET 6 is end-of-life.
@EgorBoEgorBoMay 9, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is marginally better, see https://www.diffchecker.com/Gxzd78oe
It seems that you compared codegen for constant length which is not the case here. Overall, Span.Clear should be better e.g. it's faster for small sizes as it won't pay price for the interop machinery so I assume it's better to just always use that instead of #if-else. If it's for some reason slower it can be a good motivation for us to fix that in BCL/JIT
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is not for constant length, see https://csharp.godbolt.org/z/a157vhTT9
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation for NativeMemory.Clear actually uses SpanHelpers.ClearWithoutReferences(ref *(byte*)ptr, byteCount)
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, since NativeMemory.Alloc guarantees ptr is pointer-aligned, and Length is always IntPtr aligned, we can use Unsafe.InitBlock.
@runfoapprunfoappBot
mentioned this pull request
May 9, 2023

Closed

@xtqqczze
xtqqczze
commented
May 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Test failures are unrelated and have been linked to existing issues.
@xtqqczze
xtqqczzeforce-pushed
the
NativeMemory.Odbc
branch
from
bf12034 to
b593c9bCompareMay 9, 2023 21:42
@build-analysisbuild-analysisBot
mentioned this pull request
May 10, 2023

Open

stephentoub
stephentoub
reviewed
May 15, 2023
View reviewed changes
Comment threadsrc/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
 protected DbBuffer(int initialSize) : base(IntPtr.Zero, true)
 protected unsafe DbBuffer(int initialSize) : base(IntPtr.Zero, true)
 {
 Debug.Assert(initialSize % IntPtr.Size == 0, $"Expected aligned {nameof(initialSize)}.");
@stephentoubstephentoubMay 15, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What guarantees this?
@xtqqczzextqqczzeMay 25, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DbBuffer is is only instantiated from the derived class CNativeBuffer.
The buffer size is either 4096 bytes, or a larger value calculated in CalcParameterBufferSize.
runtime/src/libraries/System.Data.Odbc/src/System/Data/Odbc/OdbcParameterCollection.cs

Lines 112 to 128
in
b593c9b
internalintCalcParameterBufferSize(OdbcCommandcommand)
{
// Calculate the size of the buffer we need
intparameterBufferSize=0;
for(inti=0;i<Count;++i)
{
if(_rebindCollection)
{
this[i].HasChanged=true;
}
this[i].PrepareForBind(command,(short)(i+1),refparameterBufferSize);
parameterBufferSize=(parameterBufferSize+(IntPtr.Size-1))&~(IntPtr.Size-1);// align buffer;
}
returnparameterBufferSize;
}
@xtqqczzextqqczzeMay 25, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it makes sense to calculate aligned buffer size in the ctor instead.
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 2, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
1efc89e
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 6, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
19a2c9f
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 6, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
2843098
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 7, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
b44c546
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 9, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
1b3a525
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 13, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
323d2ea
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 15, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
76495ad
Ref: dotnet/runtime#85996
@xtqqczze
xtqqczze
marked this pull request as draft
September 11, 2023 14:52
@build-analysisbuild-analysisBot
mentioned this pull request
Sep 11, 2023

Closed

@ghostghost
closed this
Oct 11, 2023
@ghost
ghost
commented
Oct 11, 2023

Copy link


Copy Markdown

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.
@ghostghost
locked as resolved and limited conversation to collaborators
Nov 10, 2023

This pull request was closed.

Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@EgorBoEgorBoEgorBo left review comments
@stephentoubstephentoubstephentoub left review comments

Assignees


No one assigned


Labels

area-System.Datacommunity-contributionIndicates that the PR has been added by a community member

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@xtqqczze@Wraith2@EgorBo@stephentoub

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Use `NativeMemory` in `System.Data.Odbc` by xtqqczze · Pull Request #85966 · dotnet/runtime · GitHub
Skip to content
Use NativeMemory in System.Data.Odbc - #85966
Closed
xtqqczze wants to merge 4 commits into
dotnet:mainfrom 
xtqqczze:NativeMemory.Odbc
Closed
Use NativeMemory in System.Data.Odbc#85966
xtqqczze wants to merge 4 commits into
dotnet:mainfrom 
xtqqczze:NativeMemory.Odbc
Conversation
@xtqqczze
@xtqqczzextqqczze
commented
May 9, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Contributes to #54297.
Although target frameworks for System.Data.Odbc include netstandard2.0 and $(NetFrameworkMinimum), these platforms are not supported (see #78550), therefore we can use the .NET 6 NativeMemory APIs.
@ghostghost
added

community-contribution
Indicates that the PR has been added by a community member
area-System.Data

labels
May 9, 2023
@ghost
ghost
commented
May 9, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @roji, @ajcvickers

See info in area-owners.md if you want to be subscribed.
Issue Details
Contributes to #54297.
Author:xtqqczze
Assignees:-
Labels:
area-System.Data, community-contribution
Milestone:-
@xtqqczze
xtqqczze
commented
May 9, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Related: #54468, cc: @reflectronic.
@xtqqczze
xtqqczzeforce-pushed
the
NativeMemory.Odbc
branch
from
aae5920 to
a99952dCompareMay 9, 2023 09:43
@build-analysisbuild-analysisBot
mentioned this pull request
May 9, 2023

Closed

@Wraith2
Wraith2
commented
May 9, 2023

Copy link


Copy Markdown

Contributor
Does this have any specific benefits?
@xtqqczze
xtqqczze
commented
May 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Does this have any specific benefits?
Use of LocalAlloc is deprecated and NativeMemory.Alloc is faster.
EgorBo
EgorBo
reviewed
May 9, 2023
View reviewed changes
Comment threadsrc/libraries/System.Data.Odbc/src/Common/System/Data/Common/SafeNativeMethods.cs
Outdated

 var zeroes = new byte[length];
 Marshal.Copy(zeroes, 0, ptr, length);
#if !NET7_0_OR_GREATER
 new Span<byte>((void*)ptr, length).Clear();
@EgorBoEgorBoMay 9, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just Span.Clear for all TFMs ?
@xtqqczzextqqczzeMay 9, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is marginally better, see https://www.diffchecker.com/Gxzd78oe
More significantly - the #if indicates we could just use NativeMemory.Clear once .NET 6 is end-of-life.
@EgorBoEgorBoMay 9, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is marginally better, see https://www.diffchecker.com/Gxzd78oe
It seems that you compared codegen for constant length which is not the case here. Overall, Span.Clear should be better e.g. it's faster for small sizes as it won't pay price for the interop machinery so I assume it's better to just always use that instead of #if-else. If it's for some reason slower it can be a good motivation for us to fix that in BCL/JIT
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is not for constant length, see https://csharp.godbolt.org/z/a157vhTT9
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation for NativeMemory.Clear actually uses SpanHelpers.ClearWithoutReferences(ref *(byte*)ptr, byteCount)
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, since NativeMemory.Alloc guarantees ptr is pointer-aligned, and Length is always IntPtr aligned, we can use Unsafe.InitBlock.
@runfoapprunfoappBot
mentioned this pull request
May 9, 2023

Closed

@xtqqczze
xtqqczze
commented
May 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Test failures are unrelated and have been linked to existing issues.
@xtqqczze
xtqqczzeforce-pushed
the
NativeMemory.Odbc
branch
from
bf12034 to
b593c9bCompareMay 9, 2023 21:42
@build-analysisbuild-analysisBot
mentioned this pull request
May 10, 2023

Open

stephentoub
stephentoub
reviewed
May 15, 2023
View reviewed changes
Comment threadsrc/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
 protected DbBuffer(int initialSize) : base(IntPtr.Zero, true)
 protected unsafe DbBuffer(int initialSize) : base(IntPtr.Zero, true)
 {
 Debug.Assert(initialSize % IntPtr.Size == 0, $"Expected aligned {nameof(initialSize)}.");
@stephentoubstephentoubMay 15, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What guarantees this?
@xtqqczzextqqczzeMay 25, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DbBuffer is is only instantiated from the derived class CNativeBuffer.
The buffer size is either 4096 bytes, or a larger value calculated in CalcParameterBufferSize.
runtime/src/libraries/System.Data.Odbc/src/System/Data/Odbc/OdbcParameterCollection.cs

Lines 112 to 128
in
b593c9b
internalintCalcParameterBufferSize(OdbcCommandcommand)
{
// Calculate the size of the buffer we need
intparameterBufferSize=0;
for(inti=0;i<Count;++i)
{
if(_rebindCollection)
{
this[i].HasChanged=true;
}
this[i].PrepareForBind(command,(short)(i+1),refparameterBufferSize);
parameterBufferSize=(parameterBufferSize+(IntPtr.Size-1))&~(IntPtr.Size-1);// align buffer;
}
returnparameterBufferSize;
}
@xtqqczzextqqczzeMay 25, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it makes sense to calculate aligned buffer size in the ctor instead.
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 2, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
1efc89e
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 6, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
19a2c9f
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 6, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
2843098
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 7, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
b44c546
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 9, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
1b3a525
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 13, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
323d2ea
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 15, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
76495ad
Ref: dotnet/runtime#85996
@xtqqczze
xtqqczze
marked this pull request as draft
September 11, 2023 14:52
@build-analysisbuild-analysisBot
mentioned this pull request
Sep 11, 2023

Closed

@ghostghost
closed this
Oct 11, 2023
@ghost
ghost
commented
Oct 11, 2023

Copy link


Copy Markdown

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.
@ghostghost
locked as resolved and limited conversation to collaborators
Nov 10, 2023

This pull request was closed.

Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@EgorBoEgorBoEgorBo left review comments
@stephentoubstephentoubstephentoub left review comments

Assignees


No one assigned


Labels

area-System.Datacommunity-contributionIndicates that the PR has been added by a community member

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@xtqqczze@Wraith2@EgorBo@stephentoub

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Use `NativeMemory` in `System.Data.Odbc` by xtqqczze · Pull Request #85966 · dotnet/runtime · GitHub
Skip to content
Use NativeMemory in System.Data.Odbc - #85966
Closed
xtqqczze wants to merge 4 commits into
dotnet:mainfrom 
xtqqczze:NativeMemory.Odbc
Closed
Use NativeMemory in System.Data.Odbc#85966
xtqqczze wants to merge 4 commits into
dotnet:mainfrom 
xtqqczze:NativeMemory.Odbc
Conversation
@xtqqczze
@xtqqczzextqqczze
commented
May 9, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Contributes to #54297.
Although target frameworks for System.Data.Odbc include netstandard2.0 and $(NetFrameworkMinimum), these platforms are not supported (see #78550), therefore we can use the .NET 6 NativeMemory APIs.
@ghostghost
added

community-contribution
Indicates that the PR has been added by a community member
area-System.Data

labels
May 9, 2023
@ghost
ghost
commented
May 9, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @roji, @ajcvickers

See info in area-owners.md if you want to be subscribed.
Issue Details
Contributes to #54297.
Author:xtqqczze
Assignees:-
Labels:
area-System.Data, community-contribution
Milestone:-
@xtqqczze
xtqqczze
commented
May 9, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Related: #54468, cc: @reflectronic.
@xtqqczze
xtqqczzeforce-pushed
the
NativeMemory.Odbc
branch
from
aae5920 to
a99952dCompareMay 9, 2023 09:43
@build-analysisbuild-analysisBot
mentioned this pull request
May 9, 2023

Closed

@Wraith2
Wraith2
commented
May 9, 2023

Copy link


Copy Markdown

Contributor
Does this have any specific benefits?
@xtqqczze
xtqqczze
commented
May 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Does this have any specific benefits?
Use of LocalAlloc is deprecated and NativeMemory.Alloc is faster.
EgorBo
EgorBo
reviewed
May 9, 2023
View reviewed changes
Comment threadsrc/libraries/System.Data.Odbc/src/Common/System/Data/Common/SafeNativeMethods.cs
Outdated

 var zeroes = new byte[length];
 Marshal.Copy(zeroes, 0, ptr, length);
#if !NET7_0_OR_GREATER
 new Span<byte>((void*)ptr, length).Clear();
@EgorBoEgorBoMay 9, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just Span.Clear for all TFMs ?
@xtqqczzextqqczzeMay 9, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is marginally better, see https://www.diffchecker.com/Gxzd78oe
More significantly - the #if indicates we could just use NativeMemory.Clear once .NET 6 is end-of-life.
@EgorBoEgorBoMay 9, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is marginally better, see https://www.diffchecker.com/Gxzd78oe
It seems that you compared codegen for constant length which is not the case here. Overall, Span.Clear should be better e.g. it's faster for small sizes as it won't pay price for the interop machinery so I assume it's better to just always use that instead of #if-else. If it's for some reason slower it can be a good motivation for us to fix that in BCL/JIT
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is not for constant length, see https://csharp.godbolt.org/z/a157vhTT9
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation for NativeMemory.Clear actually uses SpanHelpers.ClearWithoutReferences(ref *(byte*)ptr, byteCount)
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, since NativeMemory.Alloc guarantees ptr is pointer-aligned, and Length is always IntPtr aligned, we can use Unsafe.InitBlock.
@runfoapprunfoappBot
mentioned this pull request
May 9, 2023

Closed

@xtqqczze
xtqqczze
commented
May 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Test failures are unrelated and have been linked to existing issues.
@xtqqczze
xtqqczzeforce-pushed
the
NativeMemory.Odbc
branch
from
bf12034 to
b593c9bCompareMay 9, 2023 21:42
@build-analysisbuild-analysisBot
mentioned this pull request
May 10, 2023

Open

stephentoub
stephentoub
reviewed
May 15, 2023
View reviewed changes
Comment threadsrc/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
 protected DbBuffer(int initialSize) : base(IntPtr.Zero, true)
 protected unsafe DbBuffer(int initialSize) : base(IntPtr.Zero, true)
 {
 Debug.Assert(initialSize % IntPtr.Size == 0, $"Expected aligned {nameof(initialSize)}.");
@stephentoubstephentoubMay 15, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What guarantees this?
@xtqqczzextqqczzeMay 25, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DbBuffer is is only instantiated from the derived class CNativeBuffer.
The buffer size is either 4096 bytes, or a larger value calculated in CalcParameterBufferSize.
runtime/src/libraries/System.Data.Odbc/src/System/Data/Odbc/OdbcParameterCollection.cs

Lines 112 to 128
in
b593c9b
internalintCalcParameterBufferSize(OdbcCommandcommand)
{
// Calculate the size of the buffer we need
intparameterBufferSize=0;
for(inti=0;i<Count;++i)
{
if(_rebindCollection)
{
this[i].HasChanged=true;
}
this[i].PrepareForBind(command,(short)(i+1),refparameterBufferSize);
parameterBufferSize=(parameterBufferSize+(IntPtr.Size-1))&~(IntPtr.Size-1);// align buffer;
}
returnparameterBufferSize;
}
@xtqqczzextqqczzeMay 25, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it makes sense to calculate aligned buffer size in the ctor instead.
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 2, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
1efc89e
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 6, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
19a2c9f
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 6, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
2843098
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 7, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
b44c546
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 9, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
1b3a525
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 13, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
323d2ea
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 15, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
76495ad
Ref: dotnet/runtime#85996
@xtqqczze
xtqqczze
marked this pull request as draft
September 11, 2023 14:52
@build-analysisbuild-analysisBot
mentioned this pull request
Sep 11, 2023

Closed

@ghostghost
closed this
Oct 11, 2023
@ghost
ghost
commented
Oct 11, 2023

Copy link


Copy Markdown

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.
@ghostghost
locked as resolved and limited conversation to collaborators
Nov 10, 2023

This pull request was closed.

Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@EgorBoEgorBoEgorBo left review comments
@stephentoubstephentoubstephentoub left review comments

Assignees


No one assigned


Labels

area-System.Datacommunity-contributionIndicates that the PR has been added by a community member

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@xtqqczze@Wraith2@EgorBo@stephentoub

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
Use `NativeMemory` in `System.Data.Odbc` by xtqqczze · Pull Request #85966 · dotnet/runtime · GitHub
Skip to content
Use NativeMemory in System.Data.Odbc - #85966
Closed
xtqqczze wants to merge 4 commits into
dotnet:mainfrom 
xtqqczze:NativeMemory.Odbc
Closed
Use NativeMemory in System.Data.Odbc#85966
xtqqczze wants to merge 4 commits into
dotnet:mainfrom 
xtqqczze:NativeMemory.Odbc
Conversation
@xtqqczze
@xtqqczzextqqczze
commented
May 9, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Contributes to #54297.
Although target frameworks for System.Data.Odbc include netstandard2.0 and $(NetFrameworkMinimum), these platforms are not supported (see #78550), therefore we can use the .NET 6 NativeMemory APIs.
@ghostghost
added

community-contribution
Indicates that the PR has been added by a community member
area-System.Data

labels
May 9, 2023
@ghost
ghost
commented
May 9, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @roji, @ajcvickers

See info in area-owners.md if you want to be subscribed.
Issue Details
Contributes to #54297.
Author:xtqqczze
Assignees:-
Labels:
area-System.Data, community-contribution
Milestone:-
@xtqqczze
xtqqczze
commented
May 9, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Related: #54468, cc: @reflectronic.
@xtqqczze
xtqqczzeforce-pushed
the
NativeMemory.Odbc
branch
from
aae5920 to
a99952dCompareMay 9, 2023 09:43
@build-analysisbuild-analysisBot
mentioned this pull request
May 9, 2023

Closed

@Wraith2
Wraith2
commented
May 9, 2023

Copy link


Copy Markdown

Contributor
Does this have any specific benefits?
@xtqqczze
xtqqczze
commented
May 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Does this have any specific benefits?
Use of LocalAlloc is deprecated and NativeMemory.Alloc is faster.
EgorBo
EgorBo
reviewed
May 9, 2023
View reviewed changes
Comment threadsrc/libraries/System.Data.Odbc/src/Common/System/Data/Common/SafeNativeMethods.cs
Outdated

 var zeroes = new byte[length];
 Marshal.Copy(zeroes, 0, ptr, length);
#if !NET7_0_OR_GREATER
 new Span<byte>((void*)ptr, length).Clear();
@EgorBoEgorBoMay 9, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just Span.Clear for all TFMs ?
@xtqqczzextqqczzeMay 9, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is marginally better, see https://www.diffchecker.com/Gxzd78oe
More significantly - the #if indicates we could just use NativeMemory.Clear once .NET 6 is end-of-life.
@EgorBoEgorBoMay 9, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is marginally better, see https://www.diffchecker.com/Gxzd78oe
It seems that you compared codegen for constant length which is not the case here. Overall, Span.Clear should be better e.g. it's faster for small sizes as it won't pay price for the interop machinery so I assume it's better to just always use that instead of #if-else. If it's for some reason slower it can be a good motivation for us to fix that in BCL/JIT
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is not for constant length, see https://csharp.godbolt.org/z/a157vhTT9
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation for NativeMemory.Clear actually uses SpanHelpers.ClearWithoutReferences(ref *(byte*)ptr, byteCount)
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, since NativeMemory.Alloc guarantees ptr is pointer-aligned, and Length is always IntPtr aligned, we can use Unsafe.InitBlock.
@runfoapprunfoappBot
mentioned this pull request
May 9, 2023

Closed

@xtqqczze
xtqqczze
commented
May 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Test failures are unrelated and have been linked to existing issues.
@xtqqczze
xtqqczzeforce-pushed
the
NativeMemory.Odbc
branch
from
bf12034 to
b593c9bCompareMay 9, 2023 21:42
@build-analysisbuild-analysisBot
mentioned this pull request
May 10, 2023

Open

stephentoub
stephentoub
reviewed
May 15, 2023
View reviewed changes
Comment threadsrc/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
 protected DbBuffer(int initialSize) : base(IntPtr.Zero, true)
 protected unsafe DbBuffer(int initialSize) : base(IntPtr.Zero, true)
 {
 Debug.Assert(initialSize % IntPtr.Size == 0, $"Expected aligned {nameof(initialSize)}.");
@stephentoubstephentoubMay 15, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What guarantees this?
@xtqqczzextqqczzeMay 25, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DbBuffer is is only instantiated from the derived class CNativeBuffer.
The buffer size is either 4096 bytes, or a larger value calculated in CalcParameterBufferSize.
runtime/src/libraries/System.Data.Odbc/src/System/Data/Odbc/OdbcParameterCollection.cs

Lines 112 to 128
in
b593c9b
internalintCalcParameterBufferSize(OdbcCommandcommand)
{
// Calculate the size of the buffer we need
intparameterBufferSize=0;
for(inti=0;i<Count;++i)
{
if(_rebindCollection)
{
this[i].HasChanged=true;
}
this[i].PrepareForBind(command,(short)(i+1),refparameterBufferSize);
parameterBufferSize=(parameterBufferSize+(IntPtr.Size-1))&~(IntPtr.Size-1);// align buffer;
}
returnparameterBufferSize;
}
@xtqqczzextqqczzeMay 25, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it makes sense to calculate aligned buffer size in the ctor instead.
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 2, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
1efc89e
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 6, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
19a2c9f
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 6, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
2843098
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 7, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
b44c546
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 9, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
1b3a525
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 13, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
323d2ea
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 15, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
76495ad
Ref: dotnet/runtime#85996
@xtqqczze
xtqqczze
marked this pull request as draft
September 11, 2023 14:52
@build-analysisbuild-analysisBot
mentioned this pull request
Sep 11, 2023

Closed

@ghostghost
closed this
Oct 11, 2023
@ghost
ghost
commented
Oct 11, 2023

Copy link


Copy Markdown

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.
@ghostghost
locked as resolved and limited conversation to collaborators
Nov 10, 2023

This pull request was closed.

Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@EgorBoEgorBoEgorBo left review comments
@stephentoubstephentoubstephentoub left review comments

Assignees


No one assigned


Labels

area-System.Datacommunity-contributionIndicates that the PR has been added by a community member

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@xtqqczze@Wraith2@EgorBo@stephentoub

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Use `NativeMemory` in `System.Data.Odbc` by xtqqczze · Pull Request #85966 · dotnet/runtime · GitHub
Skip to content
Use NativeMemory in System.Data.Odbc - #85966
Closed
xtqqczze wants to merge 4 commits into
dotnet:mainfrom 
xtqqczze:NativeMemory.Odbc
Closed
Use NativeMemory in System.Data.Odbc#85966
xtqqczze wants to merge 4 commits into
dotnet:mainfrom 
xtqqczze:NativeMemory.Odbc
Conversation
@xtqqczze
@xtqqczzextqqczze
commented
May 9, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Contributes to #54297.
Although target frameworks for System.Data.Odbc include netstandard2.0 and $(NetFrameworkMinimum), these platforms are not supported (see #78550), therefore we can use the .NET 6 NativeMemory APIs.
@ghostghost
added

community-contribution
Indicates that the PR has been added by a community member
area-System.Data

labels
May 9, 2023
@ghost
ghost
commented
May 9, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @roji, @ajcvickers

See info in area-owners.md if you want to be subscribed.
Issue Details
Contributes to #54297.
Author:xtqqczze
Assignees:-
Labels:
area-System.Data, community-contribution
Milestone:-
@xtqqczze
xtqqczze
commented
May 9, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Related: #54468, cc: @reflectronic.
@xtqqczze
xtqqczzeforce-pushed
the
NativeMemory.Odbc
branch
from
aae5920 to
a99952dCompareMay 9, 2023 09:43
@build-analysisbuild-analysisBot
mentioned this pull request
May 9, 2023

Closed

@Wraith2
Wraith2
commented
May 9, 2023

Copy link


Copy Markdown

Contributor
Does this have any specific benefits?
@xtqqczze
xtqqczze
commented
May 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Does this have any specific benefits?
Use of LocalAlloc is deprecated and NativeMemory.Alloc is faster.
EgorBo
EgorBo
reviewed
May 9, 2023
View reviewed changes
Comment threadsrc/libraries/System.Data.Odbc/src/Common/System/Data/Common/SafeNativeMethods.cs
Outdated

 var zeroes = new byte[length];
 Marshal.Copy(zeroes, 0, ptr, length);
#if !NET7_0_OR_GREATER
 new Span<byte>((void*)ptr, length).Clear();
@EgorBoEgorBoMay 9, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just Span.Clear for all TFMs ?
@xtqqczzextqqczzeMay 9, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is marginally better, see https://www.diffchecker.com/Gxzd78oe
More significantly - the #if indicates we could just use NativeMemory.Clear once .NET 6 is end-of-life.
@EgorBoEgorBoMay 9, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is marginally better, see https://www.diffchecker.com/Gxzd78oe
It seems that you compared codegen for constant length which is not the case here. Overall, Span.Clear should be better e.g. it's faster for small sizes as it won't pay price for the interop machinery so I assume it's better to just always use that instead of #if-else. If it's for some reason slower it can be a good motivation for us to fix that in BCL/JIT
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is not for constant length, see https://csharp.godbolt.org/z/a157vhTT9
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation for NativeMemory.Clear actually uses SpanHelpers.ClearWithoutReferences(ref *(byte*)ptr, byteCount)
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, since NativeMemory.Alloc guarantees ptr is pointer-aligned, and Length is always IntPtr aligned, we can use Unsafe.InitBlock.
@runfoapprunfoappBot
mentioned this pull request
May 9, 2023

Closed

@xtqqczze
xtqqczze
commented
May 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Test failures are unrelated and have been linked to existing issues.
@xtqqczze
xtqqczzeforce-pushed
the
NativeMemory.Odbc
branch
from
bf12034 to
b593c9bCompareMay 9, 2023 21:42
@build-analysisbuild-analysisBot
mentioned this pull request
May 10, 2023

Open

stephentoub
stephentoub
reviewed
May 15, 2023
View reviewed changes
Comment threadsrc/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
 protected DbBuffer(int initialSize) : base(IntPtr.Zero, true)
 protected unsafe DbBuffer(int initialSize) : base(IntPtr.Zero, true)
 {
 Debug.Assert(initialSize % IntPtr.Size == 0, $"Expected aligned {nameof(initialSize)}.");
@stephentoubstephentoubMay 15, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What guarantees this?
@xtqqczzextqqczzeMay 25, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DbBuffer is is only instantiated from the derived class CNativeBuffer.
The buffer size is either 4096 bytes, or a larger value calculated in CalcParameterBufferSize.
runtime/src/libraries/System.Data.Odbc/src/System/Data/Odbc/OdbcParameterCollection.cs

Lines 112 to 128
in
b593c9b
internalintCalcParameterBufferSize(OdbcCommandcommand)
{
// Calculate the size of the buffer we need
intparameterBufferSize=0;
for(inti=0;i<Count;++i)
{
if(_rebindCollection)
{
this[i].HasChanged=true;
}
this[i].PrepareForBind(command,(short)(i+1),refparameterBufferSize);
parameterBufferSize=(parameterBufferSize+(IntPtr.Size-1))&~(IntPtr.Size-1);// align buffer;
}
returnparameterBufferSize;
}
@xtqqczzextqqczzeMay 25, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it makes sense to calculate aligned buffer size in the ctor instead.
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 2, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
1efc89e
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 6, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
19a2c9f
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 6, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
2843098
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 7, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
b44c546
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 9, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
1b3a525
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 13, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
323d2ea
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 15, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
76495ad
Ref: dotnet/runtime#85996
@xtqqczze
xtqqczze
marked this pull request as draft
September 11, 2023 14:52
@build-analysisbuild-analysisBot
mentioned this pull request
Sep 11, 2023

Closed

@ghostghost
closed this
Oct 11, 2023
@ghost
ghost
commented
Oct 11, 2023

Copy link


Copy Markdown

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.
@ghostghost
locked as resolved and limited conversation to collaborators
Nov 10, 2023

This pull request was closed.

Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@EgorBoEgorBoEgorBo left review comments
@stephentoubstephentoubstephentoub left review comments

Assignees


No one assigned


Labels

area-System.Datacommunity-contributionIndicates that the PR has been added by a community member

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@xtqqczze@Wraith2@EgorBo@stephentoub

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
})();
Use `NativeMemory` in `System.Data.Odbc` by xtqqczze · Pull Request #85966 · dotnet/runtime · GitHub
Skip to content
Use NativeMemory in System.Data.Odbc - #85966
Closed
xtqqczze wants to merge 4 commits into
dotnet:mainfrom 
xtqqczze:NativeMemory.Odbc
Closed
Use NativeMemory in System.Data.Odbc#85966
xtqqczze wants to merge 4 commits into
dotnet:mainfrom 
xtqqczze:NativeMemory.Odbc
Conversation
@xtqqczze
@xtqqczzextqqczze
commented
May 9, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Contributes to #54297.
Although target frameworks for System.Data.Odbc include netstandard2.0 and $(NetFrameworkMinimum), these platforms are not supported (see #78550), therefore we can use the .NET 6 NativeMemory APIs.
@ghostghost
added

community-contribution
Indicates that the PR has been added by a community member
area-System.Data

labels
May 9, 2023
@ghost
ghost
commented
May 9, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @roji, @ajcvickers

See info in area-owners.md if you want to be subscribed.
Issue Details
Contributes to #54297.
Author:xtqqczze
Assignees:-
Labels:
area-System.Data, community-contribution
Milestone:-
@xtqqczze
xtqqczze
commented
May 9, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Related: #54468, cc: @reflectronic.
@xtqqczze
xtqqczzeforce-pushed
the
NativeMemory.Odbc
branch
from
aae5920 to
a99952dCompareMay 9, 2023 09:43
@build-analysisbuild-analysisBot
mentioned this pull request
May 9, 2023

Closed

@Wraith2
Wraith2
commented
May 9, 2023

Copy link


Copy Markdown

Contributor
Does this have any specific benefits?
@xtqqczze
xtqqczze
commented
May 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Does this have any specific benefits?
Use of LocalAlloc is deprecated and NativeMemory.Alloc is faster.
EgorBo
EgorBo
reviewed
May 9, 2023
View reviewed changes
Comment threadsrc/libraries/System.Data.Odbc/src/Common/System/Data/Common/SafeNativeMethods.cs
Outdated

 var zeroes = new byte[length];
 Marshal.Copy(zeroes, 0, ptr, length);
#if !NET7_0_OR_GREATER
 new Span<byte>((void*)ptr, length).Clear();
@EgorBoEgorBoMay 9, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just Span.Clear for all TFMs ?
@xtqqczzextqqczzeMay 9, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is marginally better, see https://www.diffchecker.com/Gxzd78oe
More significantly - the #if indicates we could just use NativeMemory.Clear once .NET 6 is end-of-life.
@EgorBoEgorBoMay 9, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is marginally better, see https://www.diffchecker.com/Gxzd78oe
It seems that you compared codegen for constant length which is not the case here. Overall, Span.Clear should be better e.g. it's faster for small sizes as it won't pay price for the interop machinery so I assume it's better to just always use that instead of #if-else. If it's for some reason slower it can be a good motivation for us to fix that in BCL/JIT
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codegen is not for constant length, see https://csharp.godbolt.org/z/a157vhTT9
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation for NativeMemory.Clear actually uses SpanHelpers.ClearWithoutReferences(ref *(byte*)ptr, byteCount)
@xtqqczzextqqczzeMay 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, since NativeMemory.Alloc guarantees ptr is pointer-aligned, and Length is always IntPtr aligned, we can use Unsafe.InitBlock.
@runfoapprunfoappBot
mentioned this pull request
May 9, 2023

Closed

@xtqqczze
xtqqczze
commented
May 9, 2023

Copy link


Copy Markdown

ContributorAuthor
Test failures are unrelated and have been linked to existing issues.
@xtqqczze
xtqqczzeforce-pushed
the
NativeMemory.Odbc
branch
from
bf12034 to
b593c9bCompareMay 9, 2023 21:42
@build-analysisbuild-analysisBot
mentioned this pull request
May 10, 2023

Open

stephentoub
stephentoub
reviewed
May 15, 2023
View reviewed changes
Comment threadsrc/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
 protected DbBuffer(int initialSize) : base(IntPtr.Zero, true)
 protected unsafe DbBuffer(int initialSize) : base(IntPtr.Zero, true)
 {
 Debug.Assert(initialSize % IntPtr.Size == 0, $"Expected aligned {nameof(initialSize)}.");
@stephentoubstephentoubMay 15, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What guarantees this?
@xtqqczzextqqczzeMay 25, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DbBuffer is is only instantiated from the derived class CNativeBuffer.
The buffer size is either 4096 bytes, or a larger value calculated in CalcParameterBufferSize.
runtime/src/libraries/System.Data.Odbc/src/System/Data/Odbc/OdbcParameterCollection.cs

Lines 112 to 128
in
b593c9b
internalintCalcParameterBufferSize(OdbcCommandcommand)
{
// Calculate the size of the buffer we need
intparameterBufferSize=0;
for(inti=0;i<Count;++i)
{
if(_rebindCollection)
{
this[i].HasChanged=true;
}
this[i].PrepareForBind(command,(short)(i+1),refparameterBufferSize);
parameterBufferSize=(parameterBufferSize+(IntPtr.Size-1))&~(IntPtr.Size-1);// align buffer;
}
returnparameterBufferSize;
}
@xtqqczzextqqczzeMay 25, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it makes sense to calculate aligned buffer size in the ctor instead.
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 2, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
1efc89e
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 6, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
19a2c9f
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 6, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
2843098
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 7, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
b44c546
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 9, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
1b3a525
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 13, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
323d2ea
Ref: dotnet/runtime#85996
rolfbjarne added a commit
to rolfbjarne/macios
that referenced
this pull request
Jun 15, 2023
@rolfbjarne
[builds] Automatically applydotnet/runtime#85966.
76495ad
Ref: dotnet/runtime#85996
@xtqqczze
xtqqczze
marked this pull request as draft
September 11, 2023 14:52
@build-analysisbuild-analysisBot
mentioned this pull request
Sep 11, 2023

Closed

@ghostghost
closed this
Oct 11, 2023
@ghost
ghost
commented
Oct 11, 2023

Copy link


Copy Markdown

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.
@ghostghost
locked as resolved and limited conversation to collaborators
Nov 10, 2023

This pull request was closed.

Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@EgorBoEgorBoEgorBo left review comments
@stephentoubstephentoubstephentoub left review comments

Assignees


No one assigned


Labels

area-System.Datacommunity-contributionIndicates that the PR has been added by a community member

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@xtqqczze@Wraith2@EgorBo@stephentoub