From 988ca514b9cd87c7010bfafe329b9495603e9b7b Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 29 Jul 2019 18:53:00 -0700 Subject: [PATCH 01/28] serialization to IBufferWriter attempt 1 --- .../Grpc.Core.Api/SerializationContext.cs | 18 +++++ .../Internal/DefaultSerializationContext.cs | 74 +++++++++++++++++-- .../Internal/NativeMethods.Generated.cs | 33 +++++++++ src/csharp/ext/grpc_csharp_ext.c | 61 +++++++++++++++ .../runtimes/grpc_csharp_ext_dummy_stubs.c | 12 +++ .../Grpc.Core/Internal/native_methods.include | 3 + 6 files changed, 196 insertions(+), 5 deletions(-) diff --git a/src/csharp/Grpc.Core.Api/SerializationContext.cs b/src/csharp/Grpc.Core.Api/SerializationContext.cs index 9aef2adbcd5..79107040411 100644 --- a/src/csharp/Grpc.Core.Api/SerializationContext.cs +++ b/src/csharp/Grpc.Core.Api/SerializationContext.cs @@ -17,6 +17,7 @@ #endregion using System; +using System.Buffers; namespace Grpc.Core { @@ -35,5 +36,22 @@ namespace Grpc.Core { throw new NotImplementedException(); } + + /// + /// Expose serializer as buffer writer + /// + public virtual IBufferWriter GetBufferWriter() + { + throw new NotImplementedException(); + } + + /// + /// Complete the payload written so far. + /// + public virtual void Complete() + { + throw new NotImplementedException(); + + } } } diff --git a/src/csharp/Grpc.Core/Internal/DefaultSerializationContext.cs b/src/csharp/Grpc.Core/Internal/DefaultSerializationContext.cs index cceb194879e..9098850a52e 100644 --- a/src/csharp/Grpc.Core/Internal/DefaultSerializationContext.cs +++ b/src/csharp/Grpc.Core/Internal/DefaultSerializationContext.cs @@ -17,6 +17,8 @@ #endregion using Grpc.Core.Utils; +using System; +using System.Buffers; using System.Threading; namespace Grpc.Core.Internal @@ -27,7 +29,8 @@ namespace Grpc.Core.Internal new ThreadLocal(() => new DefaultSerializationContext(), false); bool isComplete; - byte[] payload; + //byte[] payload; + NativeBufferWriter bufferWriter; public DefaultSerializationContext() { @@ -38,18 +41,48 @@ namespace Grpc.Core.Internal { GrpcPreconditions.CheckState(!isComplete); this.isComplete = true; - this.payload = payload; + + GetBufferWriter(); + var destSpan = bufferWriter.GetSpan(payload.Length); + payload.AsSpan().CopyTo(destSpan); + bufferWriter.Advance(payload.Length); + bufferWriter.Complete(); + //this.payload = payload; + } + + /// + /// Expose serializer as buffer writer + /// + public override IBufferWriter GetBufferWriter() + { + if (bufferWriter == null) + { + // TODO: avoid allocation.. + bufferWriter = new NativeBufferWriter(); + } + return bufferWriter; + } + + /// + /// Complete the payload written so far. + /// + public override void Complete() + { + GrpcPreconditions.CheckState(!isComplete); + bufferWriter.Complete(); + this.isComplete = true; } - internal byte[] GetPayload() + internal SliceBufferSafeHandle GetPayload() { - return this.payload; + return bufferWriter.GetSliceBuffer(); } public void Reset() { this.isComplete = false; - this.payload = null; + //this.payload = null; + this.bufferWriter = null; } public static DefaultSerializationContext GetInitializedThreadLocal() @@ -58,5 +91,36 @@ namespace Grpc.Core.Internal instance.Reset(); return instance; } + + private class NativeBufferWriter : IBufferWriter + { + private SliceBufferSafeHandle sliceBuffer = SliceBufferSafeHandle.Create(); + + public void Advance(int count) + { + sliceBuffer.Advance(count); + } + + public Memory GetMemory(int sizeHint = 0) + { + // TODO: implement + throw new NotImplementedException(); + } + + public Span GetSpan(int sizeHint = 0) + { + return sliceBuffer.GetSpan(sizeHint); + } + + public void Complete() + { + sliceBuffer.Complete(); + } + + public SliceBufferSafeHandle GetSliceBuffer() + { + return sliceBuffer; + } + } } } diff --git a/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs b/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs index ca3dfe97ce1..b23170376d4 100644 --- a/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs +++ b/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs @@ -122,6 +122,9 @@ namespace Grpc.Core.Internal public readonly Delegates.grpcsharp_auth_context_property_iterator_delegate grpcsharp_auth_context_property_iterator; public readonly Delegates.grpcsharp_auth_property_iterator_next_delegate grpcsharp_auth_property_iterator_next; public readonly Delegates.grpcsharp_auth_context_release_delegate grpcsharp_auth_context_release; + public readonly Delegates.grpcsharp_slice_buffer_create_delegate grpcsharp_slice_buffer_create; + public readonly Delegates.grpcsharp_slice_buffer_adjust_tail_space_delegate grpcsharp_slice_buffer_adjust_tail_space; + public readonly Delegates.grpcsharp_slice_buffer_destroy_delegate grpcsharp_slice_buffer_destroy; public readonly Delegates.gprsharp_now_delegate gprsharp_now; public readonly Delegates.gprsharp_inf_future_delegate gprsharp_inf_future; public readonly Delegates.gprsharp_inf_past_delegate gprsharp_inf_past; @@ -224,6 +227,9 @@ namespace Grpc.Core.Internal this.grpcsharp_auth_context_property_iterator = GetMethodDelegate(library); this.grpcsharp_auth_property_iterator_next = GetMethodDelegate(library); this.grpcsharp_auth_context_release = GetMethodDelegate(library); + this.grpcsharp_slice_buffer_create = GetMethodDelegate(library); + this.grpcsharp_slice_buffer_adjust_tail_space = GetMethodDelegate(library); + this.grpcsharp_slice_buffer_destroy = GetMethodDelegate(library); this.gprsharp_now = GetMethodDelegate(library); this.gprsharp_inf_future = GetMethodDelegate(library); this.gprsharp_inf_past = GetMethodDelegate(library); @@ -325,6 +331,9 @@ namespace Grpc.Core.Internal this.grpcsharp_auth_context_property_iterator = DllImportsFromStaticLib.grpcsharp_auth_context_property_iterator; this.grpcsharp_auth_property_iterator_next = DllImportsFromStaticLib.grpcsharp_auth_property_iterator_next; this.grpcsharp_auth_context_release = DllImportsFromStaticLib.grpcsharp_auth_context_release; + this.grpcsharp_slice_buffer_create = DllImportsFromStaticLib.grpcsharp_slice_buffer_create; + this.grpcsharp_slice_buffer_adjust_tail_space = DllImportsFromStaticLib.grpcsharp_slice_buffer_adjust_tail_space; + this.grpcsharp_slice_buffer_destroy = DllImportsFromStaticLib.grpcsharp_slice_buffer_destroy; this.gprsharp_now = DllImportsFromStaticLib.gprsharp_now; this.gprsharp_inf_future = DllImportsFromStaticLib.gprsharp_inf_future; this.gprsharp_inf_past = DllImportsFromStaticLib.gprsharp_inf_past; @@ -426,6 +435,9 @@ namespace Grpc.Core.Internal this.grpcsharp_auth_context_property_iterator = DllImportsFromSharedLib.grpcsharp_auth_context_property_iterator; this.grpcsharp_auth_property_iterator_next = DllImportsFromSharedLib.grpcsharp_auth_property_iterator_next; this.grpcsharp_auth_context_release = DllImportsFromSharedLib.grpcsharp_auth_context_release; + this.grpcsharp_slice_buffer_create = DllImportsFromSharedLib.grpcsharp_slice_buffer_create; + this.grpcsharp_slice_buffer_adjust_tail_space = DllImportsFromSharedLib.grpcsharp_slice_buffer_adjust_tail_space; + this.grpcsharp_slice_buffer_destroy = DllImportsFromSharedLib.grpcsharp_slice_buffer_destroy; this.gprsharp_now = DllImportsFromSharedLib.gprsharp_now; this.gprsharp_inf_future = DllImportsFromSharedLib.gprsharp_inf_future; this.gprsharp_inf_past = DllImportsFromSharedLib.gprsharp_inf_past; @@ -530,6 +542,9 @@ namespace Grpc.Core.Internal public delegate AuthContextSafeHandle.NativeAuthPropertyIterator grpcsharp_auth_context_property_iterator_delegate(AuthContextSafeHandle authContext); public delegate IntPtr grpcsharp_auth_property_iterator_next_delegate(ref AuthContextSafeHandle.NativeAuthPropertyIterator iterator); // returns const auth_property* public delegate void grpcsharp_auth_context_release_delegate(IntPtr authContext); + public delegate SliceBufferSafeHandle grpcsharp_slice_buffer_create_delegate(); + public delegate IntPtr grpcsharp_slice_buffer_adjust_tail_space_delegate(SliceBufferSafeHandle sliceBuffer, UIntPtr availableTailSpace, UIntPtr requestedTailSpace); + public delegate void grpcsharp_slice_buffer_destroy_delegate(IntPtr sliceBuffer); public delegate Timespec gprsharp_now_delegate(ClockType clockType); public delegate Timespec gprsharp_inf_future_delegate(ClockType clockType); public delegate Timespec gprsharp_inf_past_delegate(ClockType clockType); @@ -812,6 +827,15 @@ namespace Grpc.Core.Internal [DllImport(ImportName)] public static extern void grpcsharp_auth_context_release(IntPtr authContext); + [DllImport(ImportName)] + public static extern SliceBufferSafeHandle grpcsharp_slice_buffer_create(); + + [DllImport(ImportName)] + public static extern IntPtr grpcsharp_slice_buffer_adjust_tail_space(SliceBufferSafeHandle sliceBuffer, UIntPtr availableTailSpace, UIntPtr requestedTailSpace); + + [DllImport(ImportName)] + public static extern void grpcsharp_slice_buffer_destroy(IntPtr sliceBuffer); + [DllImport(ImportName)] public static extern Timespec gprsharp_now(ClockType clockType); @@ -1111,6 +1135,15 @@ namespace Grpc.Core.Internal [DllImport(ImportName)] public static extern void grpcsharp_auth_context_release(IntPtr authContext); + [DllImport(ImportName)] + public static extern SliceBufferSafeHandle grpcsharp_slice_buffer_create(); + + [DllImport(ImportName)] + public static extern IntPtr grpcsharp_slice_buffer_adjust_tail_space(SliceBufferSafeHandle sliceBuffer, UIntPtr availableTailSpace, UIntPtr requestedTailSpace); + + [DllImport(ImportName)] + public static extern void grpcsharp_slice_buffer_destroy(IntPtr sliceBuffer); + [DllImport(ImportName)] public static extern Timespec gprsharp_now(ClockType clockType); diff --git a/src/csharp/ext/grpc_csharp_ext.c b/src/csharp/ext/grpc_csharp_ext.c index 02e219ead87..e271494c942 100644 --- a/src/csharp/ext/grpc_csharp_ext.c +++ b/src/csharp/ext/grpc_csharp_ext.c @@ -1182,6 +1182,67 @@ GPR_EXPORT void GPR_CALLTYPE grpcsharp_redirect_log(grpcsharp_log_func func) { typedef void(GPR_CALLTYPE* test_callback_funcptr)(int32_t success); +/* Slice buffer functionality */ +GPR_EXPORT grpc_slice_buffer* GPR_CALLTYPE +grpcsharp_slice_buffer_create() { + grpc_slice_buffer* slice_buffer = (grpc_slice_buffer*)gpr_malloc(sizeof(grpc_slice_buffer)); + grpc_slice_buffer_init(slice_buffer); + return slice_buffer; +} + +GPR_EXPORT void GPR_CALLTYPE +grpcsharp_slice_buffer_destroy(grpc_slice_buffer* buffer) { + grpc_slice_buffer_destroy(buffer); + gpr_free(buffer); +} + +GPR_EXPORT grpc_byte_buffer* GPR_CALLTYPE +grpcsharp_create_byte_buffer_from_stolen_slices(grpc_slice_buffer* slice_buffer) { + grpc_byte_buffer* bb = + (grpc_byte_buffer*)gpr_malloc(sizeof(grpc_byte_buffer)); + memset(bb, 0, sizeof(grpc_byte_buffer)); + bb->type = GRPC_BB_RAW; + bb->data.raw.compression = GRPC_COMPRESS_NONE; + bb->data.raw.slice_buffer = *slice_buffer; + // TODO: just use move slice_buffer... + + // we transferred the ownership of members from the slice buffer to the + // the internals of byte buffer, so we just overwrite the original slice buffer with + // default values. + grpc_slice_buffer_init(slice_buffer); // TODO: need to reset available_tail_space after this.. + return bb; +} + +GPR_EXPORT void* GPR_CALLTYPE +grpcsharp_slice_buffer_adjust_tail_space(grpc_slice_buffer* buffer, size_t available_tail_space, + size_t requested_tail_space) { + + // TODO: what if available_tail_space == requested_tail_space == 0 + + if (available_tail_space >= requested_tail_space) + { + // TODO: should this be allowed at all? + grpc_slice_buffer garbage; + grpc_slice_buffer_trim_end(buffer, available_tail_space - requested_tail_space, &garbage); + grpc_slice_buffer_reset_and_unref(&garbage); + } + else + { + if (available_tail_space > 0) + { + grpc_slice_buffer garbage; + grpc_slice_buffer_trim_end(buffer, available_tail_space, &garbage); + grpc_slice_buffer_reset_and_unref(&garbage); + } + + grpc_slice new_slice = grpc_slice_malloc(requested_tail_space); + grpc_slice_buffer_add(buffer, new_slice); + } + + grpc_slice* last_slice = &(buffer->slices[buffer->count - 1]); + return GRPC_SLICE_END_PTR(*last_slice) - requested_tail_space; +} + /* Version info */ GPR_EXPORT const char* GPR_CALLTYPE grpcsharp_version_string() { return grpc_version_string(); diff --git a/src/csharp/unitypackage/unitypackage_skeleton/Plugins/Grpc.Core/runtimes/grpc_csharp_ext_dummy_stubs.c b/src/csharp/unitypackage/unitypackage_skeleton/Plugins/Grpc.Core/runtimes/grpc_csharp_ext_dummy_stubs.c index 8ff26e4ca4d..bc312bc6daf 100644 --- a/src/csharp/unitypackage/unitypackage_skeleton/Plugins/Grpc.Core/runtimes/grpc_csharp_ext_dummy_stubs.c +++ b/src/csharp/unitypackage/unitypackage_skeleton/Plugins/Grpc.Core/runtimes/grpc_csharp_ext_dummy_stubs.c @@ -374,6 +374,18 @@ void grpcsharp_auth_context_release() { fprintf(stderr, "Should never reach here"); abort(); } +void grpcsharp_slice_buffer_create() { + fprintf(stderr, "Should never reach here"); + abort(); +} +void grpcsharp_slice_buffer_adjust_tail_space() { + fprintf(stderr, "Should never reach here"); + abort(); +} +void grpcsharp_slice_buffer_destroy() { + fprintf(stderr, "Should never reach here"); + abort(); +} void gprsharp_now() { fprintf(stderr, "Should never reach here"); abort(); diff --git a/templates/src/csharp/Grpc.Core/Internal/native_methods.include b/templates/src/csharp/Grpc.Core/Internal/native_methods.include index ed7c93a7479..d8f94744723 100644 --- a/templates/src/csharp/Grpc.Core/Internal/native_methods.include +++ b/templates/src/csharp/Grpc.Core/Internal/native_methods.include @@ -88,6 +88,9 @@ native_method_signatures = [ 'AuthContextSafeHandle.NativeAuthPropertyIterator grpcsharp_auth_context_property_iterator(AuthContextSafeHandle authContext)', 'IntPtr grpcsharp_auth_property_iterator_next(ref AuthContextSafeHandle.NativeAuthPropertyIterator iterator) // returns const auth_property*', 'void grpcsharp_auth_context_release(IntPtr authContext)', + 'SliceBufferSafeHandle grpcsharp_slice_buffer_create()', + 'IntPtr grpcsharp_slice_buffer_adjust_tail_space(SliceBufferSafeHandle sliceBuffer, UIntPtr availableTailSpace, UIntPtr requestedTailSpace)', + 'void grpcsharp_slice_buffer_destroy(IntPtr sliceBuffer)', 'Timespec gprsharp_now(ClockType clockType)', 'Timespec gprsharp_inf_future(ClockType clockType)', 'Timespec gprsharp_inf_past(ClockType clockType)', From 36e46234f72f0de72602931a546b8f8495072ddd Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 29 Jul 2019 19:57:08 -0700 Subject: [PATCH 02/28] IBufferWriter: tests now passing with a hack --- .../Grpc.Core/Internal/AsyncCallBase.cs | 2 +- .../Internal/NativeMethods.Generated.cs | 22 +++++++++++++++++++ src/csharp/ext/grpc_csharp_ext.c | 17 ++++++++++++++ .../runtimes/grpc_csharp_ext_dummy_stubs.c | 8 +++++++ .../Grpc.Core/Internal/native_methods.include | 2 ++ 5 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs b/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs index ac1f9066516..23cb5ccffc7 100644 --- a/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs +++ b/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs @@ -220,7 +220,7 @@ namespace Grpc.Core.Internal { context = DefaultSerializationContext.GetInitializedThreadLocal(); serializer(msg, context); - return context.GetPayload(); + return context.GetPayload().GetPayload(); } finally { diff --git a/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs b/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs index b23170376d4..dcb0f84649c 100644 --- a/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs +++ b/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs @@ -124,6 +124,8 @@ namespace Grpc.Core.Internal public readonly Delegates.grpcsharp_auth_context_release_delegate grpcsharp_auth_context_release; public readonly Delegates.grpcsharp_slice_buffer_create_delegate grpcsharp_slice_buffer_create; public readonly Delegates.grpcsharp_slice_buffer_adjust_tail_space_delegate grpcsharp_slice_buffer_adjust_tail_space; + public readonly Delegates.grpcsharp_slice_buffer_slice_count_delegate grpcsharp_slice_buffer_slice_count; + public readonly Delegates.grpcsharp_slice_buffer_slice_peek_delegate grpcsharp_slice_buffer_slice_peek; public readonly Delegates.grpcsharp_slice_buffer_destroy_delegate grpcsharp_slice_buffer_destroy; public readonly Delegates.gprsharp_now_delegate gprsharp_now; public readonly Delegates.gprsharp_inf_future_delegate gprsharp_inf_future; @@ -229,6 +231,8 @@ namespace Grpc.Core.Internal this.grpcsharp_auth_context_release = GetMethodDelegate(library); this.grpcsharp_slice_buffer_create = GetMethodDelegate(library); this.grpcsharp_slice_buffer_adjust_tail_space = GetMethodDelegate(library); + this.grpcsharp_slice_buffer_slice_count = GetMethodDelegate(library); + this.grpcsharp_slice_buffer_slice_peek = GetMethodDelegate(library); this.grpcsharp_slice_buffer_destroy = GetMethodDelegate(library); this.gprsharp_now = GetMethodDelegate(library); this.gprsharp_inf_future = GetMethodDelegate(library); @@ -333,6 +337,8 @@ namespace Grpc.Core.Internal this.grpcsharp_auth_context_release = DllImportsFromStaticLib.grpcsharp_auth_context_release; this.grpcsharp_slice_buffer_create = DllImportsFromStaticLib.grpcsharp_slice_buffer_create; this.grpcsharp_slice_buffer_adjust_tail_space = DllImportsFromStaticLib.grpcsharp_slice_buffer_adjust_tail_space; + this.grpcsharp_slice_buffer_slice_count = DllImportsFromStaticLib.grpcsharp_slice_buffer_slice_count; + this.grpcsharp_slice_buffer_slice_peek = DllImportsFromStaticLib.grpcsharp_slice_buffer_slice_peek; this.grpcsharp_slice_buffer_destroy = DllImportsFromStaticLib.grpcsharp_slice_buffer_destroy; this.gprsharp_now = DllImportsFromStaticLib.gprsharp_now; this.gprsharp_inf_future = DllImportsFromStaticLib.gprsharp_inf_future; @@ -437,6 +443,8 @@ namespace Grpc.Core.Internal this.grpcsharp_auth_context_release = DllImportsFromSharedLib.grpcsharp_auth_context_release; this.grpcsharp_slice_buffer_create = DllImportsFromSharedLib.grpcsharp_slice_buffer_create; this.grpcsharp_slice_buffer_adjust_tail_space = DllImportsFromSharedLib.grpcsharp_slice_buffer_adjust_tail_space; + this.grpcsharp_slice_buffer_slice_count = DllImportsFromSharedLib.grpcsharp_slice_buffer_slice_count; + this.grpcsharp_slice_buffer_slice_peek = DllImportsFromSharedLib.grpcsharp_slice_buffer_slice_peek; this.grpcsharp_slice_buffer_destroy = DllImportsFromSharedLib.grpcsharp_slice_buffer_destroy; this.gprsharp_now = DllImportsFromSharedLib.gprsharp_now; this.gprsharp_inf_future = DllImportsFromSharedLib.gprsharp_inf_future; @@ -544,6 +552,8 @@ namespace Grpc.Core.Internal public delegate void grpcsharp_auth_context_release_delegate(IntPtr authContext); public delegate SliceBufferSafeHandle grpcsharp_slice_buffer_create_delegate(); public delegate IntPtr grpcsharp_slice_buffer_adjust_tail_space_delegate(SliceBufferSafeHandle sliceBuffer, UIntPtr availableTailSpace, UIntPtr requestedTailSpace); + public delegate UIntPtr grpcsharp_slice_buffer_slice_count_delegate(SliceBufferSafeHandle sliceBuffer); + public delegate void grpcsharp_slice_buffer_slice_peek_delegate(SliceBufferSafeHandle sliceBuffer, UIntPtr index, out UIntPtr sliceLen, out IntPtr sliceDataPtr); public delegate void grpcsharp_slice_buffer_destroy_delegate(IntPtr sliceBuffer); public delegate Timespec gprsharp_now_delegate(ClockType clockType); public delegate Timespec gprsharp_inf_future_delegate(ClockType clockType); @@ -833,6 +843,12 @@ namespace Grpc.Core.Internal [DllImport(ImportName)] public static extern IntPtr grpcsharp_slice_buffer_adjust_tail_space(SliceBufferSafeHandle sliceBuffer, UIntPtr availableTailSpace, UIntPtr requestedTailSpace); + [DllImport(ImportName)] + public static extern UIntPtr grpcsharp_slice_buffer_slice_count(SliceBufferSafeHandle sliceBuffer); + + [DllImport(ImportName)] + public static extern void grpcsharp_slice_buffer_slice_peek(SliceBufferSafeHandle sliceBuffer, UIntPtr index, out UIntPtr sliceLen, out IntPtr sliceDataPtr); + [DllImport(ImportName)] public static extern void grpcsharp_slice_buffer_destroy(IntPtr sliceBuffer); @@ -1141,6 +1157,12 @@ namespace Grpc.Core.Internal [DllImport(ImportName)] public static extern IntPtr grpcsharp_slice_buffer_adjust_tail_space(SliceBufferSafeHandle sliceBuffer, UIntPtr availableTailSpace, UIntPtr requestedTailSpace); + [DllImport(ImportName)] + public static extern UIntPtr grpcsharp_slice_buffer_slice_count(SliceBufferSafeHandle sliceBuffer); + + [DllImport(ImportName)] + public static extern void grpcsharp_slice_buffer_slice_peek(SliceBufferSafeHandle sliceBuffer, UIntPtr index, out UIntPtr sliceLen, out IntPtr sliceDataPtr); + [DllImport(ImportName)] public static extern void grpcsharp_slice_buffer_destroy(IntPtr sliceBuffer); diff --git a/src/csharp/ext/grpc_csharp_ext.c b/src/csharp/ext/grpc_csharp_ext.c index e271494c942..1f115b1caf2 100644 --- a/src/csharp/ext/grpc_csharp_ext.c +++ b/src/csharp/ext/grpc_csharp_ext.c @@ -1196,6 +1196,19 @@ grpcsharp_slice_buffer_destroy(grpc_slice_buffer* buffer) { gpr_free(buffer); } +GPR_EXPORT size_t GPR_CALLTYPE +grpcsharp_slice_buffer_slice_count(grpc_slice_buffer* buffer) { + return buffer->count; +} + +GPR_EXPORT void GPR_CALLTYPE +grpcsharp_slice_buffer_slice_peek(grpc_slice_buffer* buffer, size_t index, size_t* slice_len, uint8_t** slice_data_ptr) { + GPR_ASSERT(buffer->count > index); + grpc_slice* slice_ptr = &buffer->slices[index]; + *slice_len = GRPC_SLICE_LENGTH(*slice_ptr); + *slice_data_ptr = GRPC_SLICE_START_PTR(*slice_ptr); +} + GPR_EXPORT grpc_byte_buffer* GPR_CALLTYPE grpcsharp_create_byte_buffer_from_stolen_slices(grpc_slice_buffer* slice_buffer) { grpc_byte_buffer* bb = @@ -1217,6 +1230,10 @@ GPR_EXPORT void* GPR_CALLTYPE grpcsharp_slice_buffer_adjust_tail_space(grpc_slice_buffer* buffer, size_t available_tail_space, size_t requested_tail_space) { + if (available_tail_space == 0 && requested_tail_space == 0) + { + return NULL; + } // TODO: what if available_tail_space == requested_tail_space == 0 if (available_tail_space >= requested_tail_space) diff --git a/src/csharp/unitypackage/unitypackage_skeleton/Plugins/Grpc.Core/runtimes/grpc_csharp_ext_dummy_stubs.c b/src/csharp/unitypackage/unitypackage_skeleton/Plugins/Grpc.Core/runtimes/grpc_csharp_ext_dummy_stubs.c index bc312bc6daf..9007d7f3ad0 100644 --- a/src/csharp/unitypackage/unitypackage_skeleton/Plugins/Grpc.Core/runtimes/grpc_csharp_ext_dummy_stubs.c +++ b/src/csharp/unitypackage/unitypackage_skeleton/Plugins/Grpc.Core/runtimes/grpc_csharp_ext_dummy_stubs.c @@ -382,6 +382,14 @@ void grpcsharp_slice_buffer_adjust_tail_space() { fprintf(stderr, "Should never reach here"); abort(); } +void grpcsharp_slice_buffer_slice_count() { + fprintf(stderr, "Should never reach here"); + abort(); +} +void grpcsharp_slice_buffer_slice_peek() { + fprintf(stderr, "Should never reach here"); + abort(); +} void grpcsharp_slice_buffer_destroy() { fprintf(stderr, "Should never reach here"); abort(); diff --git a/templates/src/csharp/Grpc.Core/Internal/native_methods.include b/templates/src/csharp/Grpc.Core/Internal/native_methods.include index d8f94744723..055af4a7a27 100644 --- a/templates/src/csharp/Grpc.Core/Internal/native_methods.include +++ b/templates/src/csharp/Grpc.Core/Internal/native_methods.include @@ -90,6 +90,8 @@ native_method_signatures = [ 'void grpcsharp_auth_context_release(IntPtr authContext)', 'SliceBufferSafeHandle grpcsharp_slice_buffer_create()', 'IntPtr grpcsharp_slice_buffer_adjust_tail_space(SliceBufferSafeHandle sliceBuffer, UIntPtr availableTailSpace, UIntPtr requestedTailSpace)', + 'UIntPtr grpcsharp_slice_buffer_slice_count(SliceBufferSafeHandle sliceBuffer)', + 'void grpcsharp_slice_buffer_slice_peek(SliceBufferSafeHandle sliceBuffer, UIntPtr index, out UIntPtr sliceLen, out IntPtr sliceDataPtr)', 'void grpcsharp_slice_buffer_destroy(IntPtr sliceBuffer)', 'Timespec gprsharp_now(ClockType clockType)', 'Timespec gprsharp_inf_future(ClockType clockType)', From 1154c2d17ea07d0b95dd6f4ac8776c4f3841570e Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 29 Jul 2019 19:58:36 -0700 Subject: [PATCH 03/28] fixup: add SliceBufferSafeHandle --- .../Internal/SliceBufferSafeHandle.cs | 122 ++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs diff --git a/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs b/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs new file mode 100644 index 00000000000..ef9e7fbce37 --- /dev/null +++ b/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs @@ -0,0 +1,122 @@ +#region Copyright notice and license + +// Copyright 2019 The gRPC Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#endregion + +using System; +using System.Runtime.InteropServices; +using Grpc.Core; +using Grpc.Core.Logging; +using Grpc.Core.Utils; + +namespace Grpc.Core.Internal +{ + /// + /// grpc_slice_buffer + /// + internal class SliceBufferSafeHandle : SafeHandleZeroIsInvalid + { + static readonly NativeMethods Native = NativeMethods.Get(); + static readonly ILogger Logger = GrpcEnvironment.Logger.ForType(); + + private IntPtr tailSpacePtr; + private UIntPtr tailSpaceLen; + + + private SliceBufferSafeHandle() + { + } + + public static SliceBufferSafeHandle Create() + { + return Native.grpcsharp_slice_buffer_create(); + } + + public IntPtr Handle + { + get + { + return handle; + } + } + + public void Advance(int count) + { + GrpcPreconditions.CheckArgument(count >= 0); + GrpcPreconditions.CheckArgument(tailSpacePtr != IntPtr.Zero || count == 0); + GrpcPreconditions.CheckArgument(tailSpaceLen.ToUInt64() >= (ulong)count); + tailSpaceLen = new UIntPtr(tailSpaceLen.ToUInt64() - (ulong)count); + tailSpacePtr += count; + } + + public unsafe Span GetSpan(int sizeHint) + { + GrpcPreconditions.CheckArgument(sizeHint >= 0); + if (tailSpaceLen.ToUInt64() < (ulong) sizeHint) + { + // TODO: should we ignore the hint sometimes when + // available tail space is close enough to the sizeHint? + AdjustTailSpace(sizeHint); + } + return new Span(tailSpacePtr.ToPointer(), (int) tailSpaceLen.ToUInt64()); + } + + public void Complete() + { + AdjustTailSpace(0); + } + + public byte[] GetPayload() + { + ulong sliceCount = Native.grpcsharp_slice_buffer_slice_count(this).ToUInt64(); + + Slice[] slices = new Slice[sliceCount]; + int totalLen = 0; + for (int i = 0; i < (int) sliceCount; i++) + { + Native.grpcsharp_slice_buffer_slice_peek(this, new UIntPtr((ulong) i), out UIntPtr sliceLen, out IntPtr dataPtr); + slices[i] = new Slice(dataPtr, (int) sliceLen.ToUInt64()); + totalLen += (int) sliceLen.ToUInt64(); + + } + var result = new byte[totalLen]; + int offset = 0; + for (int i = 0; i < (int) sliceCount; i++) + { + slices[i].ToSpanUnsafe().CopyTo(result.AsSpan(offset, slices[i].Length)); + offset += slices[i].Length; + } + GrpcPreconditions.CheckState(totalLen == offset); + return result; + } + // TODO: converting contents to byte[] + + // Gets data of server_rpc_new completion. + private void AdjustTailSpace(int requestedSize) + { + GrpcPreconditions.CheckArgument(requestedSize >= 0); + var requestedTailSpaceLen = new UIntPtr((ulong) requestedSize); + tailSpacePtr = Native.grpcsharp_slice_buffer_adjust_tail_space(this, tailSpaceLen, requestedTailSpaceLen); + tailSpaceLen = requestedTailSpaceLen; + } + + protected override bool ReleaseHandle() + { + Native.grpcsharp_slice_buffer_destroy(handle); + return true; + } + } +} From b9183153211d109ba57598812ee8d90a12236443 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 30 Jul 2019 16:16:56 -0700 Subject: [PATCH 04/28] a bit of refactor --- src/csharp/Grpc.Core/Internal/AsyncCallBase.cs | 2 +- src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs b/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs index 23cb5ccffc7..b26b7921955 100644 --- a/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs +++ b/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs @@ -220,7 +220,7 @@ namespace Grpc.Core.Internal { context = DefaultSerializationContext.GetInitializedThreadLocal(); serializer(msg, context); - return context.GetPayload().GetPayload(); + return context.GetPayload().ToByteArray(); } finally { diff --git a/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs b/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs index ef9e7fbce37..5b540c69d44 100644 --- a/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs +++ b/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs @@ -79,7 +79,9 @@ namespace Grpc.Core.Internal AdjustTailSpace(0); } - public byte[] GetPayload() + // copies the content of the slice buffer to a newly allocated byte array + // due to its overhead, this method should only be used for testing. + public byte[] ToByteArray() { ulong sliceCount = Native.grpcsharp_slice_buffer_slice_count(this).ToUInt64(); @@ -102,7 +104,6 @@ namespace Grpc.Core.Internal GrpcPreconditions.CheckState(totalLen == offset); return result; } - // TODO: converting contents to byte[] // Gets data of server_rpc_new completion. private void AdjustTailSpace(int requestedSize) @@ -112,7 +113,6 @@ namespace Grpc.Core.Internal tailSpacePtr = Native.grpcsharp_slice_buffer_adjust_tail_space(this, tailSpaceLen, requestedTailSpaceLen); tailSpaceLen = requestedTailSpaceLen; } - protected override bool ReleaseHandle() { Native.grpcsharp_slice_buffer_destroy(handle); From 31eff3e679f12f3db28c80527f7f376c1beb8efd Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 30 Jul 2019 17:16:49 -0700 Subject: [PATCH 05/28] stuff builds but tests crash --- .../Internal/FakeNativeCall.cs | 10 +-- src/csharp/Grpc.Core/Internal/AsyncCall.cs | 6 +- .../Grpc.Core/Internal/AsyncCallBase.cs | 6 +- .../Grpc.Core/Internal/AsyncCallServer.cs | 2 +- .../Grpc.Core/Internal/CallSafeHandle.cs | 26 +++---- src/csharp/Grpc.Core/Internal/INativeCall.cs | 10 +-- .../Internal/NativeMethods.Generated.cs | 30 ++++----- .../SendMessageBenchmark.cs | 5 +- .../UnaryCallOverheadBenchmark.cs | 4 +- src/csharp/Grpc.Microbenchmarks/Utf8Encode.cs | 2 +- src/csharp/ext/grpc_csharp_ext.c | 67 ++++++++++--------- .../Grpc.Core/Internal/native_methods.include | 10 +-- 12 files changed, 91 insertions(+), 87 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/Internal/FakeNativeCall.cs b/src/csharp/Grpc.Core.Tests/Internal/FakeNativeCall.cs index ef67918dabb..c16b55ca941 100644 --- a/src/csharp/Grpc.Core.Tests/Internal/FakeNativeCall.cs +++ b/src/csharp/Grpc.Core.Tests/Internal/FakeNativeCall.cs @@ -101,13 +101,13 @@ namespace Grpc.Core.Internal.Tests return "PEER"; } - public void StartUnary(IUnaryResponseClientCallback callback, byte[] payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags) + public void StartUnary(IUnaryResponseClientCallback callback, SliceBufferSafeHandle payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags) { StartCallMaybeFail(); UnaryResponseClientCallback = callback; } - public void StartUnary(BatchContextSafeHandle ctx, byte[] payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags) + public void StartUnary(BatchContextSafeHandle ctx, SliceBufferSafeHandle payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags) { StartCallMaybeFail(); throw new NotImplementedException(); @@ -119,7 +119,7 @@ namespace Grpc.Core.Internal.Tests UnaryResponseClientCallback = callback; } - public void StartServerStreaming(IReceivedStatusOnClientCallback callback, byte[] payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags) + public void StartServerStreaming(IReceivedStatusOnClientCallback callback, SliceBufferSafeHandle payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags) { StartCallMaybeFail(); ReceivedStatusOnClientCallback = callback; @@ -146,7 +146,7 @@ namespace Grpc.Core.Internal.Tests SendCompletionCallback = callback; } - public void StartSendMessage(ISendCompletionCallback callback, byte[] payload, WriteFlags writeFlags, bool sendEmptyInitialMetadata) + public void StartSendMessage(ISendCompletionCallback callback, SliceBufferSafeHandle payload, WriteFlags writeFlags, bool sendEmptyInitialMetadata) { SendCompletionCallback = callback; } @@ -157,7 +157,7 @@ namespace Grpc.Core.Internal.Tests } public void StartSendStatusFromServer(ISendStatusFromServerCompletionCallback callback, Status status, MetadataArraySafeHandle metadataArray, bool sendEmptyInitialMetadata, - byte[] optionalPayload, WriteFlags writeFlags) + SliceBufferSafeHandle payload, WriteFlags writeFlags) { SendStatusFromServerCallback = callback; } diff --git a/src/csharp/Grpc.Core/Internal/AsyncCall.cs b/src/csharp/Grpc.Core/Internal/AsyncCall.cs index ce3508d398e..aefa58a0cee 100644 --- a/src/csharp/Grpc.Core/Internal/AsyncCall.cs +++ b/src/csharp/Grpc.Core/Internal/AsyncCall.cs @@ -95,7 +95,7 @@ namespace Grpc.Core.Internal readingDone = true; } - byte[] payload = UnsafeSerialize(msg); + var payload = UnsafeSerialize(msg); using (var metadataArray = MetadataArraySafeHandle.Create(details.Options.Headers)) { @@ -160,7 +160,7 @@ namespace Grpc.Core.Internal halfcloseRequested = true; readingDone = true; - byte[] payload = UnsafeSerialize(msg); + var payload = UnsafeSerialize(msg); unaryResponseTcs = new TaskCompletionSource(); using (var metadataArray = MetadataArraySafeHandle.Create(details.Options.Headers)) @@ -235,7 +235,7 @@ namespace Grpc.Core.Internal halfcloseRequested = true; - byte[] payload = UnsafeSerialize(msg); + var payload = UnsafeSerialize(msg); streamingResponseCallFinishedTcs = new TaskCompletionSource(); using (var metadataArray = MetadataArraySafeHandle.Create(details.Options.Headers)) diff --git a/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs b/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs index b26b7921955..07f9ecf23e9 100644 --- a/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs +++ b/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs @@ -115,7 +115,7 @@ namespace Grpc.Core.Internal /// protected Task SendMessageInternalAsync(TWrite msg, WriteFlags writeFlags) { - byte[] payload = UnsafeSerialize(msg); + var payload = UnsafeSerialize(msg); lock (myLock) { @@ -213,14 +213,14 @@ namespace Grpc.Core.Internal /// protected abstract Task CheckSendAllowedOrEarlyResult(); - protected byte[] UnsafeSerialize(TWrite msg) + protected SliceBufferSafeHandle UnsafeSerialize(TWrite msg) { DefaultSerializationContext context = null; try { context = DefaultSerializationContext.GetInitializedThreadLocal(); serializer(msg, context); - return context.GetPayload().ToByteArray(); + return context.GetPayload(); } finally { diff --git a/src/csharp/Grpc.Core/Internal/AsyncCallServer.cs b/src/csharp/Grpc.Core/Internal/AsyncCallServer.cs index 0bf1fb3b7dd..e1c3a215422 100644 --- a/src/csharp/Grpc.Core/Internal/AsyncCallServer.cs +++ b/src/csharp/Grpc.Core/Internal/AsyncCallServer.cs @@ -129,7 +129,7 @@ namespace Grpc.Core.Internal /// public Task SendStatusFromServerAsync(Status status, Metadata trailers, ResponseWithFlags? optionalWrite) { - byte[] payload = optionalWrite.HasValue ? UnsafeSerialize(optionalWrite.Value.Response) : null; + var payload = optionalWrite.HasValue ? UnsafeSerialize(optionalWrite.Value.Response) : null; var writeFlags = optionalWrite.HasValue ? optionalWrite.Value.WriteFlags : default(WriteFlags); lock (myLock) diff --git a/src/csharp/Grpc.Core/Internal/CallSafeHandle.cs b/src/csharp/Grpc.Core/Internal/CallSafeHandle.cs index 2e637c9704b..6208f45a00a 100644 --- a/src/csharp/Grpc.Core/Internal/CallSafeHandle.cs +++ b/src/csharp/Grpc.Core/Internal/CallSafeHandle.cs @@ -67,19 +67,19 @@ namespace Grpc.Core.Internal Native.grpcsharp_call_set_credentials(this, credentials).CheckOk(); } - public void StartUnary(IUnaryResponseClientCallback callback, byte[] payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags) + public void StartUnary(IUnaryResponseClientCallback callback, SliceBufferSafeHandle payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags) { using (completionQueue.NewScope()) { var ctx = completionQueue.CompletionRegistry.RegisterBatchCompletion(CompletionHandler_IUnaryResponseClientCallback, callback); - Native.grpcsharp_call_start_unary(this, ctx, payload, new UIntPtr((ulong)payload.Length), writeFlags, metadataArray, callFlags) + Native.grpcsharp_call_start_unary(this, ctx, payload, writeFlags, metadataArray, callFlags) .CheckOk(); } } - public void StartUnary(BatchContextSafeHandle ctx, byte[] payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags) + public void StartUnary(BatchContextSafeHandle ctx, SliceBufferSafeHandle payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags) { - Native.grpcsharp_call_start_unary(this, ctx, payload, new UIntPtr((ulong)payload.Length), writeFlags, metadataArray, callFlags) + Native.grpcsharp_call_start_unary(this, ctx, payload, writeFlags, metadataArray, callFlags) .CheckOk(); } @@ -92,12 +92,12 @@ namespace Grpc.Core.Internal } } - public void StartServerStreaming(IReceivedStatusOnClientCallback callback, byte[] payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags) + public void StartServerStreaming(IReceivedStatusOnClientCallback callback, SliceBufferSafeHandle payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags) { using (completionQueue.NewScope()) { var ctx = completionQueue.CompletionRegistry.RegisterBatchCompletion(CompletionHandler_IReceivedStatusOnClientCallback, callback); - Native.grpcsharp_call_start_server_streaming(this, ctx, payload, new UIntPtr((ulong)payload.Length), writeFlags, metadataArray, callFlags).CheckOk(); + Native.grpcsharp_call_start_server_streaming(this, ctx, payload, writeFlags, metadataArray, callFlags).CheckOk(); } } @@ -110,12 +110,12 @@ namespace Grpc.Core.Internal } } - public void StartSendMessage(ISendCompletionCallback callback, byte[] payload, WriteFlags writeFlags, bool sendEmptyInitialMetadata) + public void StartSendMessage(ISendCompletionCallback callback, SliceBufferSafeHandle payload, WriteFlags writeFlags, bool sendEmptyInitialMetadata) { using (completionQueue.NewScope()) { var ctx = completionQueue.CompletionRegistry.RegisterBatchCompletion(CompletionHandler_ISendCompletionCallback, callback); - Native.grpcsharp_call_send_message(this, ctx, payload, new UIntPtr((ulong)payload.Length), writeFlags, sendEmptyInitialMetadata ? 1 : 0).CheckOk(); + Native.grpcsharp_call_send_message(this, ctx, payload, writeFlags, sendEmptyInitialMetadata ? 1 : 0).CheckOk(); } } @@ -129,13 +129,13 @@ namespace Grpc.Core.Internal } public void StartSendStatusFromServer(ISendStatusFromServerCompletionCallback callback, Status status, MetadataArraySafeHandle metadataArray, bool sendEmptyInitialMetadata, - byte[] optionalPayload, WriteFlags writeFlags) + SliceBufferSafeHandle optionalPayload, WriteFlags writeFlags) { + // TODO: optionalPayload == null -> pass null SliceBufferSafeHandle using (completionQueue.NewScope()) { var ctx = completionQueue.CompletionRegistry.RegisterBatchCompletion(CompletionHandler_ISendStatusFromServerCompletionCallback, callback); - var optionalPayloadLength = optionalPayload != null ? new UIntPtr((ulong)optionalPayload.Length) : UIntPtr.Zero; - + const int MaxStackAllocBytes = 256; int maxBytes = MarshalUtils.GetMaxByteCountUTF8(status.Detail); if (maxBytes > MaxStackAllocBytes) @@ -156,7 +156,7 @@ namespace Grpc.Core.Internal byte* ptr = stackalloc byte[maxBytes]; int statusBytes = MarshalUtils.GetBytesUTF8(status.Detail, ptr, maxBytes); Native.grpcsharp_call_send_status_from_server(this, ctx, status.StatusCode, new IntPtr(ptr), new UIntPtr((ulong)statusBytes), metadataArray, sendEmptyInitialMetadata ? 1 : 0, - optionalPayload, optionalPayloadLength, writeFlags).CheckOk(); + optionalPayload, writeFlags).CheckOk(); } else { // for larger status (rare), rent a buffer from the pool and @@ -168,7 +168,7 @@ namespace Grpc.Core.Internal { int statusBytes = MarshalUtils.GetBytesUTF8(status.Detail, ptr, maxBytes); Native.grpcsharp_call_send_status_from_server(this, ctx, status.StatusCode, new IntPtr(ptr), new UIntPtr((ulong)statusBytes), metadataArray, sendEmptyInitialMetadata ? 1 : 0, - optionalPayload, optionalPayloadLength, writeFlags).CheckOk(); + optionalPayload, writeFlags).CheckOk(); } } finally diff --git a/src/csharp/Grpc.Core/Internal/INativeCall.cs b/src/csharp/Grpc.Core/Internal/INativeCall.cs index 98117c6988a..70e98bfc9af 100644 --- a/src/csharp/Grpc.Core/Internal/INativeCall.cs +++ b/src/csharp/Grpc.Core/Internal/INativeCall.cs @@ -67,13 +67,13 @@ namespace Grpc.Core.Internal string GetPeer(); - void StartUnary(IUnaryResponseClientCallback callback, byte[] payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags); + void StartUnary(IUnaryResponseClientCallback callback, SliceBufferSafeHandle payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags); - void StartUnary(BatchContextSafeHandle ctx, byte[] payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags); + void StartUnary(BatchContextSafeHandle ctx, SliceBufferSafeHandle payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags); void StartClientStreaming(IUnaryResponseClientCallback callback, MetadataArraySafeHandle metadataArray, CallFlags callFlags); - void StartServerStreaming(IReceivedStatusOnClientCallback callback, byte[] payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags); + void StartServerStreaming(IReceivedStatusOnClientCallback callback, SliceBufferSafeHandle payload, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags callFlags); void StartDuplexStreaming(IReceivedStatusOnClientCallback callback, MetadataArraySafeHandle metadataArray, CallFlags callFlags); @@ -83,11 +83,11 @@ namespace Grpc.Core.Internal void StartSendInitialMetadata(ISendCompletionCallback callback, MetadataArraySafeHandle metadataArray); - void StartSendMessage(ISendCompletionCallback callback, byte[] payload, WriteFlags writeFlags, bool sendEmptyInitialMetadata); + void StartSendMessage(ISendCompletionCallback callback, SliceBufferSafeHandle payload, WriteFlags writeFlags, bool sendEmptyInitialMetadata); void StartSendCloseFromClient(ISendCompletionCallback callback); - void StartSendStatusFromServer(ISendStatusFromServerCompletionCallback callback, Status status, MetadataArraySafeHandle metadataArray, bool sendEmptyInitialMetadata, byte[] optionalPayload, WriteFlags writeFlags); + void StartSendStatusFromServer(ISendStatusFromServerCompletionCallback callback, Status status, MetadataArraySafeHandle metadataArray, bool sendEmptyInitialMetadata, SliceBufferSafeHandle optionalPayload, WriteFlags writeFlags); void StartServerSide(IReceivedCloseOnServerCallback callback); } diff --git a/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs b/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs index dcb0f84649c..d492d0c9525 100644 --- a/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs +++ b/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs @@ -487,13 +487,13 @@ namespace Grpc.Core.Internal public delegate void grpcsharp_call_credentials_release_delegate(IntPtr credentials); public delegate CallError grpcsharp_call_cancel_delegate(CallSafeHandle call); public delegate CallError grpcsharp_call_cancel_with_status_delegate(CallSafeHandle call, StatusCode status, string description); - public delegate CallError grpcsharp_call_start_unary_delegate(CallSafeHandle call, BatchContextSafeHandle ctx, byte[] sendBuffer, UIntPtr sendBufferLen, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); + public delegate CallError grpcsharp_call_start_unary_delegate(CallSafeHandle call, BatchContextSafeHandle ctx, SliceBufferSafeHandle sendBuffer, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); public delegate CallError grpcsharp_call_start_client_streaming_delegate(CallSafeHandle call, BatchContextSafeHandle ctx, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); - public delegate CallError grpcsharp_call_start_server_streaming_delegate(CallSafeHandle call, BatchContextSafeHandle ctx, byte[] sendBuffer, UIntPtr sendBufferLen, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); + public delegate CallError grpcsharp_call_start_server_streaming_delegate(CallSafeHandle call, BatchContextSafeHandle ctx, SliceBufferSafeHandle sendBuffer, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); public delegate CallError grpcsharp_call_start_duplex_streaming_delegate(CallSafeHandle call, BatchContextSafeHandle ctx, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); - public delegate CallError grpcsharp_call_send_message_delegate(CallSafeHandle call, BatchContextSafeHandle ctx, byte[] sendBuffer, UIntPtr sendBufferLen, WriteFlags writeFlags, int sendEmptyInitialMetadata); + public delegate CallError grpcsharp_call_send_message_delegate(CallSafeHandle call, BatchContextSafeHandle ctx, SliceBufferSafeHandle sendBuffer, WriteFlags writeFlags, int sendEmptyInitialMetadata); public delegate CallError grpcsharp_call_send_close_from_client_delegate(CallSafeHandle call, BatchContextSafeHandle ctx); - public delegate CallError grpcsharp_call_send_status_from_server_delegate(CallSafeHandle call, BatchContextSafeHandle ctx, StatusCode statusCode, IntPtr statusMessage, UIntPtr statusMessageLen, MetadataArraySafeHandle metadataArray, int sendEmptyInitialMetadata, byte[] optionalSendBuffer, UIntPtr optionalSendBufferLen, WriteFlags writeFlags); + public delegate CallError grpcsharp_call_send_status_from_server_delegate(CallSafeHandle call, BatchContextSafeHandle ctx, StatusCode statusCode, IntPtr statusMessage, UIntPtr statusMessageLen, MetadataArraySafeHandle metadataArray, int sendEmptyInitialMetadata, SliceBufferSafeHandle optionalSendBuffer, WriteFlags writeFlags); public delegate CallError grpcsharp_call_recv_message_delegate(CallSafeHandle call, BatchContextSafeHandle ctx); public delegate CallError grpcsharp_call_recv_initial_metadata_delegate(CallSafeHandle call, BatchContextSafeHandle ctx); public delegate CallError grpcsharp_call_start_serverside_delegate(CallSafeHandle call, BatchContextSafeHandle ctx); @@ -563,7 +563,7 @@ namespace Grpc.Core.Internal public delegate CallError grpcsharp_test_callback_delegate([MarshalAs(UnmanagedType.FunctionPtr)] NativeCallbackTestDelegate callback); public delegate IntPtr grpcsharp_test_nop_delegate(IntPtr ptr); public delegate void grpcsharp_test_override_method_delegate(string methodName, string variant); - public delegate CallError grpcsharp_test_call_start_unary_echo_delegate(CallSafeHandle call, BatchContextSafeHandle ctx, byte[] sendBuffer, UIntPtr sendBufferLen, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); + public delegate CallError grpcsharp_test_call_start_unary_echo_delegate(CallSafeHandle call, BatchContextSafeHandle ctx, SliceBufferSafeHandle sendBuffer, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); } /// @@ -649,25 +649,25 @@ namespace Grpc.Core.Internal public static extern CallError grpcsharp_call_cancel_with_status(CallSafeHandle call, StatusCode status, string description); [DllImport(ImportName)] - public static extern CallError grpcsharp_call_start_unary(CallSafeHandle call, BatchContextSafeHandle ctx, byte[] sendBuffer, UIntPtr sendBufferLen, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); + public static extern CallError grpcsharp_call_start_unary(CallSafeHandle call, BatchContextSafeHandle ctx, SliceBufferSafeHandle sendBuffer, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); [DllImport(ImportName)] public static extern CallError grpcsharp_call_start_client_streaming(CallSafeHandle call, BatchContextSafeHandle ctx, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); [DllImport(ImportName)] - public static extern CallError grpcsharp_call_start_server_streaming(CallSafeHandle call, BatchContextSafeHandle ctx, byte[] sendBuffer, UIntPtr sendBufferLen, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); + public static extern CallError grpcsharp_call_start_server_streaming(CallSafeHandle call, BatchContextSafeHandle ctx, SliceBufferSafeHandle sendBuffer, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); [DllImport(ImportName)] public static extern CallError grpcsharp_call_start_duplex_streaming(CallSafeHandle call, BatchContextSafeHandle ctx, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); [DllImport(ImportName)] - public static extern CallError grpcsharp_call_send_message(CallSafeHandle call, BatchContextSafeHandle ctx, byte[] sendBuffer, UIntPtr sendBufferLen, WriteFlags writeFlags, int sendEmptyInitialMetadata); + public static extern CallError grpcsharp_call_send_message(CallSafeHandle call, BatchContextSafeHandle ctx, SliceBufferSafeHandle sendBuffer, WriteFlags writeFlags, int sendEmptyInitialMetadata); [DllImport(ImportName)] public static extern CallError grpcsharp_call_send_close_from_client(CallSafeHandle call, BatchContextSafeHandle ctx); [DllImport(ImportName)] - public static extern CallError grpcsharp_call_send_status_from_server(CallSafeHandle call, BatchContextSafeHandle ctx, StatusCode statusCode, IntPtr statusMessage, UIntPtr statusMessageLen, MetadataArraySafeHandle metadataArray, int sendEmptyInitialMetadata, byte[] optionalSendBuffer, UIntPtr optionalSendBufferLen, WriteFlags writeFlags); + public static extern CallError grpcsharp_call_send_status_from_server(CallSafeHandle call, BatchContextSafeHandle ctx, StatusCode statusCode, IntPtr statusMessage, UIntPtr statusMessageLen, MetadataArraySafeHandle metadataArray, int sendEmptyInitialMetadata, SliceBufferSafeHandle optionalSendBuffer, WriteFlags writeFlags); [DllImport(ImportName)] public static extern CallError grpcsharp_call_recv_message(CallSafeHandle call, BatchContextSafeHandle ctx); @@ -877,7 +877,7 @@ namespace Grpc.Core.Internal public static extern void grpcsharp_test_override_method(string methodName, string variant); [DllImport(ImportName)] - public static extern CallError grpcsharp_test_call_start_unary_echo(CallSafeHandle call, BatchContextSafeHandle ctx, byte[] sendBuffer, UIntPtr sendBufferLen, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); + public static extern CallError grpcsharp_test_call_start_unary_echo(CallSafeHandle call, BatchContextSafeHandle ctx, SliceBufferSafeHandle sendBuffer, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); } /// @@ -963,25 +963,25 @@ namespace Grpc.Core.Internal public static extern CallError grpcsharp_call_cancel_with_status(CallSafeHandle call, StatusCode status, string description); [DllImport(ImportName)] - public static extern CallError grpcsharp_call_start_unary(CallSafeHandle call, BatchContextSafeHandle ctx, byte[] sendBuffer, UIntPtr sendBufferLen, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); + public static extern CallError grpcsharp_call_start_unary(CallSafeHandle call, BatchContextSafeHandle ctx, SliceBufferSafeHandle sendBuffer, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); [DllImport(ImportName)] public static extern CallError grpcsharp_call_start_client_streaming(CallSafeHandle call, BatchContextSafeHandle ctx, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); [DllImport(ImportName)] - public static extern CallError grpcsharp_call_start_server_streaming(CallSafeHandle call, BatchContextSafeHandle ctx, byte[] sendBuffer, UIntPtr sendBufferLen, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); + public static extern CallError grpcsharp_call_start_server_streaming(CallSafeHandle call, BatchContextSafeHandle ctx, SliceBufferSafeHandle sendBuffer, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); [DllImport(ImportName)] public static extern CallError grpcsharp_call_start_duplex_streaming(CallSafeHandle call, BatchContextSafeHandle ctx, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); [DllImport(ImportName)] - public static extern CallError grpcsharp_call_send_message(CallSafeHandle call, BatchContextSafeHandle ctx, byte[] sendBuffer, UIntPtr sendBufferLen, WriteFlags writeFlags, int sendEmptyInitialMetadata); + public static extern CallError grpcsharp_call_send_message(CallSafeHandle call, BatchContextSafeHandle ctx, SliceBufferSafeHandle sendBuffer, WriteFlags writeFlags, int sendEmptyInitialMetadata); [DllImport(ImportName)] public static extern CallError grpcsharp_call_send_close_from_client(CallSafeHandle call, BatchContextSafeHandle ctx); [DllImport(ImportName)] - public static extern CallError grpcsharp_call_send_status_from_server(CallSafeHandle call, BatchContextSafeHandle ctx, StatusCode statusCode, IntPtr statusMessage, UIntPtr statusMessageLen, MetadataArraySafeHandle metadataArray, int sendEmptyInitialMetadata, byte[] optionalSendBuffer, UIntPtr optionalSendBufferLen, WriteFlags writeFlags); + public static extern CallError grpcsharp_call_send_status_from_server(CallSafeHandle call, BatchContextSafeHandle ctx, StatusCode statusCode, IntPtr statusMessage, UIntPtr statusMessageLen, MetadataArraySafeHandle metadataArray, int sendEmptyInitialMetadata, SliceBufferSafeHandle optionalSendBuffer, WriteFlags writeFlags); [DllImport(ImportName)] public static extern CallError grpcsharp_call_recv_message(CallSafeHandle call, BatchContextSafeHandle ctx); @@ -1191,7 +1191,7 @@ namespace Grpc.Core.Internal public static extern void grpcsharp_test_override_method(string methodName, string variant); [DllImport(ImportName)] - public static extern CallError grpcsharp_test_call_start_unary_echo(CallSafeHandle call, BatchContextSafeHandle ctx, byte[] sendBuffer, UIntPtr sendBufferLen, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); + public static extern CallError grpcsharp_test_call_start_unary_echo(CallSafeHandle call, BatchContextSafeHandle ctx, SliceBufferSafeHandle sendBuffer, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags); } } } diff --git a/src/csharp/Grpc.Microbenchmarks/SendMessageBenchmark.cs b/src/csharp/Grpc.Microbenchmarks/SendMessageBenchmark.cs index 8496813efb0..76c59ca579c 100644 --- a/src/csharp/Grpc.Microbenchmarks/SendMessageBenchmark.cs +++ b/src/csharp/Grpc.Microbenchmarks/SendMessageBenchmark.cs @@ -50,11 +50,14 @@ namespace Grpc.Microbenchmarks var call = CreateFakeCall(cq); var sendCompletionCallback = new NopSendCompletionCallback(); - var payload = new byte[PayloadSize]; + var payload = SliceBufferSafeHandle.Create(); + payload.GetSpan(PayloadSize); + payload.Advance(PayloadSize); var writeFlags = default(WriteFlags); for (int i = 0; i < Iterations; i++) { + // TODO: sending for the first time actually steals the slices... call.StartSendMessage(sendCompletionCallback, payload, writeFlags, false); var callback = completionRegistry.Extract(completionRegistry.LastRegisteredKey); callback.OnComplete(true); diff --git a/src/csharp/Grpc.Microbenchmarks/UnaryCallOverheadBenchmark.cs b/src/csharp/Grpc.Microbenchmarks/UnaryCallOverheadBenchmark.cs index 8448f03dd62..177a49d9223 100644 --- a/src/csharp/Grpc.Microbenchmarks/UnaryCallOverheadBenchmark.cs +++ b/src/csharp/Grpc.Microbenchmarks/UnaryCallOverheadBenchmark.cs @@ -70,8 +70,8 @@ namespace Grpc.Microbenchmarks var native = NativeMethods.Get(); // replace the implementation of a native method with a fake - NativeMethods.Delegates.grpcsharp_call_start_unary_delegate fakeCallStartUnary = (CallSafeHandle call, BatchContextSafeHandle ctx, byte[] sendBuffer, UIntPtr sendBufferLen, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags) => { - return native.grpcsharp_test_call_start_unary_echo(call, ctx, sendBuffer, sendBufferLen, writeFlags, metadataArray, metadataFlags); + NativeMethods.Delegates.grpcsharp_call_start_unary_delegate fakeCallStartUnary = (CallSafeHandle call, BatchContextSafeHandle ctx, SliceBufferSafeHandle sendBuffer, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags) => { + return native.grpcsharp_test_call_start_unary_echo(call, ctx, sendBuffer, writeFlags, metadataArray, metadataFlags); }; native.GetType().GetField(nameof(native.grpcsharp_call_start_unary)).SetValue(native, fakeCallStartUnary); diff --git a/src/csharp/Grpc.Microbenchmarks/Utf8Encode.cs b/src/csharp/Grpc.Microbenchmarks/Utf8Encode.cs index a59afa19118..e5bac1967c9 100644 --- a/src/csharp/Grpc.Microbenchmarks/Utf8Encode.cs +++ b/src/csharp/Grpc.Microbenchmarks/Utf8Encode.cs @@ -61,7 +61,7 @@ namespace Grpc.Microbenchmarks var native = NativeMethods.Get(); // nop the native-call via reflection - NativeMethods.Delegates.grpcsharp_call_send_status_from_server_delegate nop = (CallSafeHandle call, BatchContextSafeHandle ctx, StatusCode statusCode, IntPtr statusMessage, UIntPtr statusMessageLen, MetadataArraySafeHandle metadataArray, int sendEmptyInitialMetadata, byte[] optionalSendBuffer, UIntPtr optionalSendBufferLen, WriteFlags writeFlags) => { + NativeMethods.Delegates.grpcsharp_call_send_status_from_server_delegate nop = (CallSafeHandle call, BatchContextSafeHandle ctx, StatusCode statusCode, IntPtr statusMessage, UIntPtr statusMessageLen, MetadataArraySafeHandle metadataArray, int sendEmptyInitialMetadata, SliceBufferSafeHandle optionalSendBuffer, WriteFlags writeFlags) => { completionRegistry.Extract(ctx.Handle).OnComplete(true); // drain the dictionary as we go return CallError.OK; }; diff --git a/src/csharp/ext/grpc_csharp_ext.c b/src/csharp/ext/grpc_csharp_ext.c index 1f115b1caf2..abddbc0e7a1 100644 --- a/src/csharp/ext/grpc_csharp_ext.c +++ b/src/csharp/ext/grpc_csharp_ext.c @@ -41,10 +41,26 @@ #define GPR_CALLTYPE #endif -grpc_byte_buffer* string_to_byte_buffer(const char* buffer, size_t len) { - grpc_slice slice = grpc_slice_from_copied_buffer(buffer, len); - grpc_byte_buffer* bb = grpc_raw_byte_buffer_create(&slice, 1); - grpc_slice_unref(slice); +//grpc_byte_buffer* string_to_byte_buffer(const char* buffer, size_t len) { +// grpc_slice slice = grpc_slice_from_copied_buffer(buffer, len); +// grpc_byte_buffer* bb = grpc_raw_byte_buffer_create(&slice, 1); +// grpc_slice_unref(slice); +// return bb; +//} + +static grpc_byte_buffer* grpcsharp_create_byte_buffer_from_stolen_slices(grpc_slice_buffer* slice_buffer) { + grpc_byte_buffer* bb = + (grpc_byte_buffer*)gpr_malloc(sizeof(grpc_byte_buffer)); + memset(bb, 0, sizeof(grpc_byte_buffer)); + bb->type = GRPC_BB_RAW; + bb->data.raw.compression = GRPC_COMPRESS_NONE; + bb->data.raw.slice_buffer = *slice_buffer; + // TODO: just use move slice_buffer... + + // we transferred the ownership of members from the slice buffer to the + // the internals of byte buffer, so we just overwrite the original slice buffer with + // default values. + grpc_slice_buffer_init(slice_buffer); // TODO: need to reset available_tail_space after this.. return bb; } @@ -582,8 +598,8 @@ static grpc_call_error grpcsharp_call_start_batch(grpc_call* call, } GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_unary( - grpc_call* call, grpcsharp_batch_context* ctx, const char* send_buffer, - size_t send_buffer_len, uint32_t write_flags, + grpc_call* call, grpcsharp_batch_context* ctx, grpc_slice_buffer* send_buffer, + uint32_t write_flags, grpc_metadata_array* initial_metadata, uint32_t initial_metadata_flags) { /* TODO: don't use magic number */ grpc_op ops[6]; @@ -598,7 +614,7 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_unary( ops[0].reserved = NULL; ops[1].op = GRPC_OP_SEND_MESSAGE; - ctx->send_message = string_to_byte_buffer(send_buffer, send_buffer_len); + ctx->send_message = grpcsharp_create_byte_buffer_from_stolen_slices(send_buffer); ops[1].data.send_message.send_message = ctx->send_message; ops[1].flags = write_flags; ops[1].reserved = NULL; @@ -635,12 +651,12 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_unary( /* Only for testing. Shortcircuits the unary call logic and only echoes the message as if it was received from the server */ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_test_call_start_unary_echo( - grpc_call* call, grpcsharp_batch_context* ctx, const char* send_buffer, - size_t send_buffer_len, uint32_t write_flags, + grpc_call* call, grpcsharp_batch_context* ctx, grpc_slice_buffer* send_buffer, + uint32_t write_flags, grpc_metadata_array* initial_metadata, uint32_t initial_metadata_flags) { // prepare as if we were performing a normal RPC. grpc_byte_buffer* send_message = - string_to_byte_buffer(send_buffer, send_buffer_len); + grpcsharp_create_byte_buffer_from_stolen_slices(send_buffer); ctx->recv_message = send_message; // echo message sent by the client as if // received from server. @@ -693,8 +709,8 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_client_streaming( } GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_server_streaming( - grpc_call* call, grpcsharp_batch_context* ctx, const char* send_buffer, - size_t send_buffer_len, uint32_t write_flags, + grpc_call* call, grpcsharp_batch_context* ctx, grpc_slice_buffer* send_buffer, + uint32_t write_flags, grpc_metadata_array* initial_metadata, uint32_t initial_metadata_flags) { /* TODO: don't use magic number */ grpc_op ops[4]; @@ -709,7 +725,7 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_server_streaming( ops[0].reserved = NULL; ops[1].op = GRPC_OP_SEND_MESSAGE; - ctx->send_message = string_to_byte_buffer(send_buffer, send_buffer_len); + ctx->send_message = grpcsharp_create_byte_buffer_from_stolen_slices(send_buffer); ops[1].data.send_message.send_message = ctx->send_message; ops[1].flags = write_flags; ops[1].reserved = NULL; @@ -776,15 +792,15 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_recv_initial_metadata( } GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_send_message( - grpc_call* call, grpcsharp_batch_context* ctx, const char* send_buffer, - size_t send_buffer_len, uint32_t write_flags, + grpc_call* call, grpcsharp_batch_context* ctx, grpc_slice_buffer* send_buffer, + uint32_t write_flags, int32_t send_empty_initial_metadata) { /* TODO: don't use magic number */ grpc_op ops[2]; memset(ops, 0, sizeof(ops)); size_t nops = send_empty_initial_metadata ? 2 : 1; ops[0].op = GRPC_OP_SEND_MESSAGE; - ctx->send_message = string_to_byte_buffer(send_buffer, send_buffer_len); + ctx->send_message = grpcsharp_create_byte_buffer_from_stolen_slices(send_buffer); ops[0].data.send_message.send_message = ctx->send_message; ops[0].flags = write_flags; ops[0].reserved = NULL; @@ -811,7 +827,7 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_send_status_from_server( grpc_call* call, grpcsharp_batch_context* ctx, grpc_status_code status_code, const char* status_details, size_t status_details_len, grpc_metadata_array* trailing_metadata, int32_t send_empty_initial_metadata, - const char* optional_send_buffer, size_t optional_send_buffer_len, + grpc_slice_buffer* optional_send_buffer, uint32_t write_flags) { /* TODO: don't use magic number */ grpc_op ops[3]; @@ -833,7 +849,7 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_send_status_from_server( if (optional_send_buffer) { ops[nops].op = GRPC_OP_SEND_MESSAGE; ctx->send_message = - string_to_byte_buffer(optional_send_buffer, optional_send_buffer_len); + grpcsharp_create_byte_buffer_from_stolen_slices(optional_send_buffer); ops[nops].data.send_message.send_message = ctx->send_message; ops[nops].flags = write_flags; ops[nops].reserved = NULL; @@ -1209,22 +1225,7 @@ grpcsharp_slice_buffer_slice_peek(grpc_slice_buffer* buffer, size_t index, size_ *slice_data_ptr = GRPC_SLICE_START_PTR(*slice_ptr); } -GPR_EXPORT grpc_byte_buffer* GPR_CALLTYPE -grpcsharp_create_byte_buffer_from_stolen_slices(grpc_slice_buffer* slice_buffer) { - grpc_byte_buffer* bb = - (grpc_byte_buffer*)gpr_malloc(sizeof(grpc_byte_buffer)); - memset(bb, 0, sizeof(grpc_byte_buffer)); - bb->type = GRPC_BB_RAW; - bb->data.raw.compression = GRPC_COMPRESS_NONE; - bb->data.raw.slice_buffer = *slice_buffer; - // TODO: just use move slice_buffer... - // we transferred the ownership of members from the slice buffer to the - // the internals of byte buffer, so we just overwrite the original slice buffer with - // default values. - grpc_slice_buffer_init(slice_buffer); // TODO: need to reset available_tail_space after this.. - return bb; -} GPR_EXPORT void* GPR_CALLTYPE grpcsharp_slice_buffer_adjust_tail_space(grpc_slice_buffer* buffer, size_t available_tail_space, diff --git a/templates/src/csharp/Grpc.Core/Internal/native_methods.include b/templates/src/csharp/Grpc.Core/Internal/native_methods.include index 055af4a7a27..4f5f61d332c 100644 --- a/templates/src/csharp/Grpc.Core/Internal/native_methods.include +++ b/templates/src/csharp/Grpc.Core/Internal/native_methods.include @@ -25,13 +25,13 @@ native_method_signatures = [ 'void grpcsharp_call_credentials_release(IntPtr credentials)', 'CallError grpcsharp_call_cancel(CallSafeHandle call)', 'CallError grpcsharp_call_cancel_with_status(CallSafeHandle call, StatusCode status, string description)', - 'CallError grpcsharp_call_start_unary(CallSafeHandle call, BatchContextSafeHandle ctx, byte[] sendBuffer, UIntPtr sendBufferLen, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags)', + 'CallError grpcsharp_call_start_unary(CallSafeHandle call, BatchContextSafeHandle ctx, SliceBufferSafeHandle sendBuffer, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags)', 'CallError grpcsharp_call_start_client_streaming(CallSafeHandle call, BatchContextSafeHandle ctx, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags)', - 'CallError grpcsharp_call_start_server_streaming(CallSafeHandle call, BatchContextSafeHandle ctx, byte[] sendBuffer, UIntPtr sendBufferLen, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags)', + 'CallError grpcsharp_call_start_server_streaming(CallSafeHandle call, BatchContextSafeHandle ctx, SliceBufferSafeHandle sendBuffer, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags)', 'CallError grpcsharp_call_start_duplex_streaming(CallSafeHandle call, BatchContextSafeHandle ctx, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags)', - 'CallError grpcsharp_call_send_message(CallSafeHandle call, BatchContextSafeHandle ctx, byte[] sendBuffer, UIntPtr sendBufferLen, WriteFlags writeFlags, int sendEmptyInitialMetadata)', + 'CallError grpcsharp_call_send_message(CallSafeHandle call, BatchContextSafeHandle ctx, SliceBufferSafeHandle sendBuffer, WriteFlags writeFlags, int sendEmptyInitialMetadata)', 'CallError grpcsharp_call_send_close_from_client(CallSafeHandle call, BatchContextSafeHandle ctx)', - 'CallError grpcsharp_call_send_status_from_server(CallSafeHandle call, BatchContextSafeHandle ctx, StatusCode statusCode, IntPtr statusMessage, UIntPtr statusMessageLen, MetadataArraySafeHandle metadataArray, int sendEmptyInitialMetadata, byte[] optionalSendBuffer, UIntPtr optionalSendBufferLen, WriteFlags writeFlags)', + 'CallError grpcsharp_call_send_status_from_server(CallSafeHandle call, BatchContextSafeHandle ctx, StatusCode statusCode, IntPtr statusMessage, UIntPtr statusMessageLen, MetadataArraySafeHandle metadataArray, int sendEmptyInitialMetadata, SliceBufferSafeHandle optionalSendBuffer, WriteFlags writeFlags)', 'CallError grpcsharp_call_recv_message(CallSafeHandle call, BatchContextSafeHandle ctx)', 'CallError grpcsharp_call_recv_initial_metadata(CallSafeHandle call, BatchContextSafeHandle ctx)', 'CallError grpcsharp_call_start_serverside(CallSafeHandle call, BatchContextSafeHandle ctx)', @@ -101,7 +101,7 @@ native_method_signatures = [ 'CallError grpcsharp_test_callback([MarshalAs(UnmanagedType.FunctionPtr)] NativeCallbackTestDelegate callback)', 'IntPtr grpcsharp_test_nop(IntPtr ptr)', 'void grpcsharp_test_override_method(string methodName, string variant)', - 'CallError grpcsharp_test_call_start_unary_echo(CallSafeHandle call, BatchContextSafeHandle ctx, byte[] sendBuffer, UIntPtr sendBufferLen, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags)', + 'CallError grpcsharp_test_call_start_unary_echo(CallSafeHandle call, BatchContextSafeHandle ctx, SliceBufferSafeHandle sendBuffer, WriteFlags writeFlags, MetadataArraySafeHandle metadataArray, CallFlags metadataFlags)', ] import re From 8ad2b146ed5caa995c89726de59c375935145cb4 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 30 Jul 2019 17:38:08 -0700 Subject: [PATCH 06/28] fix grpcsharp_create_byte_buffer_from_stolen_slices --- src/csharp/ext/grpc_csharp_ext.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/csharp/ext/grpc_csharp_ext.c b/src/csharp/ext/grpc_csharp_ext.c index abddbc0e7a1..82fa81abf7d 100644 --- a/src/csharp/ext/grpc_csharp_ext.c +++ b/src/csharp/ext/grpc_csharp_ext.c @@ -41,26 +41,15 @@ #define GPR_CALLTYPE #endif -//grpc_byte_buffer* string_to_byte_buffer(const char* buffer, size_t len) { -// grpc_slice slice = grpc_slice_from_copied_buffer(buffer, len); -// grpc_byte_buffer* bb = grpc_raw_byte_buffer_create(&slice, 1); -// grpc_slice_unref(slice); -// return bb; -//} - static grpc_byte_buffer* grpcsharp_create_byte_buffer_from_stolen_slices(grpc_slice_buffer* slice_buffer) { grpc_byte_buffer* bb = - (grpc_byte_buffer*)gpr_malloc(sizeof(grpc_byte_buffer)); + (grpc_byte_buffer*)gpr_malloc(sizeof(grpc_byte_buffer)); memset(bb, 0, sizeof(grpc_byte_buffer)); bb->type = GRPC_BB_RAW; bb->data.raw.compression = GRPC_COMPRESS_NONE; - bb->data.raw.slice_buffer = *slice_buffer; - // TODO: just use move slice_buffer... + grpc_slice_buffer_init(&bb->data.raw.slice_buffer); - // we transferred the ownership of members from the slice buffer to the - // the internals of byte buffer, so we just overwrite the original slice buffer with - // default values. - grpc_slice_buffer_init(slice_buffer); // TODO: need to reset available_tail_space after this.. + grpc_slice_buffer_swap(&bb->data.raw.slice_buffer, slice_buffer); return bb; } From 70c7aa162354188f25aa790bdc7a7443c00f2af1 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 30 Jul 2019 18:07:12 -0700 Subject: [PATCH 07/28] stuff now works --- src/csharp/Grpc.Core/Internal/CallSafeHandle.cs | 6 ++++++ src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs | 2 ++ 2 files changed, 8 insertions(+) diff --git a/src/csharp/Grpc.Core/Internal/CallSafeHandle.cs b/src/csharp/Grpc.Core/Internal/CallSafeHandle.cs index 6208f45a00a..cfcab5dc692 100644 --- a/src/csharp/Grpc.Core/Internal/CallSafeHandle.cs +++ b/src/csharp/Grpc.Core/Internal/CallSafeHandle.cs @@ -132,6 +132,12 @@ namespace Grpc.Core.Internal SliceBufferSafeHandle optionalPayload, WriteFlags writeFlags) { // TODO: optionalPayload == null -> pass null SliceBufferSafeHandle + // do this properly + if (optionalPayload == null) + { + optionalPayload = SliceBufferSafeHandle.NullInstance; + } + using (completionQueue.NewScope()) { var ctx = completionQueue.CompletionRegistry.RegisterBatchCompletion(CompletionHandler_ISendStatusFromServerCompletionCallback, callback); diff --git a/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs b/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs index 5b540c69d44..5ea4432684c 100644 --- a/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs +++ b/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs @@ -32,6 +32,8 @@ namespace Grpc.Core.Internal static readonly NativeMethods Native = NativeMethods.Get(); static readonly ILogger Logger = GrpcEnvironment.Logger.ForType(); + public static readonly SliceBufferSafeHandle NullInstance = new SliceBufferSafeHandle(); + private IntPtr tailSpacePtr; private UIntPtr tailSpaceLen; From 1c177fff1e9cba4fc3632e12d39572a31fa2704f Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 31 Jul 2019 16:22:50 -0700 Subject: [PATCH 08/28] refactoring and simplifications --- .../Internal/DefaultSerializationContext.cs | 51 ++++--------------- .../Internal/NativeMethods.Generated.cs | 11 ++++ .../Internal/SliceBufferSafeHandle.cs | 44 +++++++++++----- src/csharp/ext/grpc_csharp_ext.c | 7 ++- .../runtimes/grpc_csharp_ext_dummy_stubs.c | 4 ++ .../Grpc.Core/Internal/native_methods.include | 1 + 6 files changed, 63 insertions(+), 55 deletions(-) diff --git a/src/csharp/Grpc.Core/Internal/DefaultSerializationContext.cs b/src/csharp/Grpc.Core/Internal/DefaultSerializationContext.cs index 9098850a52e..db5e78a608d 100644 --- a/src/csharp/Grpc.Core/Internal/DefaultSerializationContext.cs +++ b/src/csharp/Grpc.Core/Internal/DefaultSerializationContext.cs @@ -30,7 +30,7 @@ namespace Grpc.Core.Internal bool isComplete; //byte[] payload; - NativeBufferWriter bufferWriter; + SliceBufferSafeHandle sliceBuffer; public DefaultSerializationContext() { @@ -43,10 +43,10 @@ namespace Grpc.Core.Internal this.isComplete = true; GetBufferWriter(); - var destSpan = bufferWriter.GetSpan(payload.Length); + var destSpan = sliceBuffer.GetSpan(payload.Length); payload.AsSpan().CopyTo(destSpan); - bufferWriter.Advance(payload.Length); - bufferWriter.Complete(); + sliceBuffer.Advance(payload.Length); + sliceBuffer.Complete(); //this.payload = payload; } @@ -55,12 +55,12 @@ namespace Grpc.Core.Internal /// public override IBufferWriter GetBufferWriter() { - if (bufferWriter == null) + if (sliceBuffer == null) { // TODO: avoid allocation.. - bufferWriter = new NativeBufferWriter(); + sliceBuffer = SliceBufferSafeHandle.Create(); } - return bufferWriter; + return sliceBuffer; } /// @@ -69,20 +69,20 @@ namespace Grpc.Core.Internal public override void Complete() { GrpcPreconditions.CheckState(!isComplete); - bufferWriter.Complete(); + sliceBuffer.Complete(); this.isComplete = true; } internal SliceBufferSafeHandle GetPayload() { - return bufferWriter.GetSliceBuffer(); + return sliceBuffer; } public void Reset() { this.isComplete = false; //this.payload = null; - this.bufferWriter = null; + this.sliceBuffer = null; // reset instead... } public static DefaultSerializationContext GetInitializedThreadLocal() @@ -92,35 +92,6 @@ namespace Grpc.Core.Internal return instance; } - private class NativeBufferWriter : IBufferWriter - { - private SliceBufferSafeHandle sliceBuffer = SliceBufferSafeHandle.Create(); - - public void Advance(int count) - { - sliceBuffer.Advance(count); - } - - public Memory GetMemory(int sizeHint = 0) - { - // TODO: implement - throw new NotImplementedException(); - } - - public Span GetSpan(int sizeHint = 0) - { - return sliceBuffer.GetSpan(sizeHint); - } - - public void Complete() - { - sliceBuffer.Complete(); - } - - public SliceBufferSafeHandle GetSliceBuffer() - { - return sliceBuffer; - } - } + } } diff --git a/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs b/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs index d492d0c9525..c724b30ca8f 100644 --- a/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs +++ b/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs @@ -126,6 +126,7 @@ namespace Grpc.Core.Internal public readonly Delegates.grpcsharp_slice_buffer_adjust_tail_space_delegate grpcsharp_slice_buffer_adjust_tail_space; public readonly Delegates.grpcsharp_slice_buffer_slice_count_delegate grpcsharp_slice_buffer_slice_count; public readonly Delegates.grpcsharp_slice_buffer_slice_peek_delegate grpcsharp_slice_buffer_slice_peek; + public readonly Delegates.grpcsharp_slice_buffer_reset_and_unref_delegate grpcsharp_slice_buffer_reset_and_unref; public readonly Delegates.grpcsharp_slice_buffer_destroy_delegate grpcsharp_slice_buffer_destroy; public readonly Delegates.gprsharp_now_delegate gprsharp_now; public readonly Delegates.gprsharp_inf_future_delegate gprsharp_inf_future; @@ -233,6 +234,7 @@ namespace Grpc.Core.Internal this.grpcsharp_slice_buffer_adjust_tail_space = GetMethodDelegate(library); this.grpcsharp_slice_buffer_slice_count = GetMethodDelegate(library); this.grpcsharp_slice_buffer_slice_peek = GetMethodDelegate(library); + this.grpcsharp_slice_buffer_reset_and_unref = GetMethodDelegate(library); this.grpcsharp_slice_buffer_destroy = GetMethodDelegate(library); this.gprsharp_now = GetMethodDelegate(library); this.gprsharp_inf_future = GetMethodDelegate(library); @@ -339,6 +341,7 @@ namespace Grpc.Core.Internal this.grpcsharp_slice_buffer_adjust_tail_space = DllImportsFromStaticLib.grpcsharp_slice_buffer_adjust_tail_space; this.grpcsharp_slice_buffer_slice_count = DllImportsFromStaticLib.grpcsharp_slice_buffer_slice_count; this.grpcsharp_slice_buffer_slice_peek = DllImportsFromStaticLib.grpcsharp_slice_buffer_slice_peek; + this.grpcsharp_slice_buffer_reset_and_unref = DllImportsFromStaticLib.grpcsharp_slice_buffer_reset_and_unref; this.grpcsharp_slice_buffer_destroy = DllImportsFromStaticLib.grpcsharp_slice_buffer_destroy; this.gprsharp_now = DllImportsFromStaticLib.gprsharp_now; this.gprsharp_inf_future = DllImportsFromStaticLib.gprsharp_inf_future; @@ -445,6 +448,7 @@ namespace Grpc.Core.Internal this.grpcsharp_slice_buffer_adjust_tail_space = DllImportsFromSharedLib.grpcsharp_slice_buffer_adjust_tail_space; this.grpcsharp_slice_buffer_slice_count = DllImportsFromSharedLib.grpcsharp_slice_buffer_slice_count; this.grpcsharp_slice_buffer_slice_peek = DllImportsFromSharedLib.grpcsharp_slice_buffer_slice_peek; + this.grpcsharp_slice_buffer_reset_and_unref = DllImportsFromSharedLib.grpcsharp_slice_buffer_reset_and_unref; this.grpcsharp_slice_buffer_destroy = DllImportsFromSharedLib.grpcsharp_slice_buffer_destroy; this.gprsharp_now = DllImportsFromSharedLib.gprsharp_now; this.gprsharp_inf_future = DllImportsFromSharedLib.gprsharp_inf_future; @@ -554,6 +558,7 @@ namespace Grpc.Core.Internal public delegate IntPtr grpcsharp_slice_buffer_adjust_tail_space_delegate(SliceBufferSafeHandle sliceBuffer, UIntPtr availableTailSpace, UIntPtr requestedTailSpace); public delegate UIntPtr grpcsharp_slice_buffer_slice_count_delegate(SliceBufferSafeHandle sliceBuffer); public delegate void grpcsharp_slice_buffer_slice_peek_delegate(SliceBufferSafeHandle sliceBuffer, UIntPtr index, out UIntPtr sliceLen, out IntPtr sliceDataPtr); + public delegate void grpcsharp_slice_buffer_reset_and_unref_delegate(SliceBufferSafeHandle sliceBuffer); public delegate void grpcsharp_slice_buffer_destroy_delegate(IntPtr sliceBuffer); public delegate Timespec gprsharp_now_delegate(ClockType clockType); public delegate Timespec gprsharp_inf_future_delegate(ClockType clockType); @@ -849,6 +854,9 @@ namespace Grpc.Core.Internal [DllImport(ImportName)] public static extern void grpcsharp_slice_buffer_slice_peek(SliceBufferSafeHandle sliceBuffer, UIntPtr index, out UIntPtr sliceLen, out IntPtr sliceDataPtr); + [DllImport(ImportName)] + public static extern void grpcsharp_slice_buffer_reset_and_unref(SliceBufferSafeHandle sliceBuffer); + [DllImport(ImportName)] public static extern void grpcsharp_slice_buffer_destroy(IntPtr sliceBuffer); @@ -1163,6 +1171,9 @@ namespace Grpc.Core.Internal [DllImport(ImportName)] public static extern void grpcsharp_slice_buffer_slice_peek(SliceBufferSafeHandle sliceBuffer, UIntPtr index, out UIntPtr sliceLen, out IntPtr sliceDataPtr); + [DllImport(ImportName)] + public static extern void grpcsharp_slice_buffer_reset_and_unref(SliceBufferSafeHandle sliceBuffer); + [DllImport(ImportName)] public static extern void grpcsharp_slice_buffer_destroy(IntPtr sliceBuffer); diff --git a/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs b/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs index 5ea4432684c..76a97f4a0a2 100644 --- a/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs +++ b/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs @@ -17,6 +17,7 @@ #endregion using System; +using System.Buffers; using System.Runtime.InteropServices; using Grpc.Core; using Grpc.Core.Logging; @@ -25,9 +26,10 @@ using Grpc.Core.Utils; namespace Grpc.Core.Internal { /// - /// grpc_slice_buffer + /// Represents grpc_slice_buffer with some extra utility functions to allow + /// writing data to it using the IBufferWriter interface. /// - internal class SliceBufferSafeHandle : SafeHandleZeroIsInvalid + internal class SliceBufferSafeHandle : SafeHandleZeroIsInvalid, IBufferWriter { static readonly NativeMethods Native = NativeMethods.Get(); static readonly ILogger Logger = GrpcEnvironment.Logger.ForType(); @@ -35,8 +37,7 @@ namespace Grpc.Core.Internal public static readonly SliceBufferSafeHandle NullInstance = new SliceBufferSafeHandle(); private IntPtr tailSpacePtr; - private UIntPtr tailSpaceLen; - + private int tailSpaceLen; private SliceBufferSafeHandle() { @@ -59,21 +60,30 @@ namespace Grpc.Core.Internal { GrpcPreconditions.CheckArgument(count >= 0); GrpcPreconditions.CheckArgument(tailSpacePtr != IntPtr.Zero || count == 0); - GrpcPreconditions.CheckArgument(tailSpaceLen.ToUInt64() >= (ulong)count); - tailSpaceLen = new UIntPtr(tailSpaceLen.ToUInt64() - (ulong)count); + GrpcPreconditions.CheckArgument(tailSpaceLen >= count); + tailSpaceLen = tailSpaceLen - count; tailSpacePtr += count; } - public unsafe Span GetSpan(int sizeHint) + // provides access to the "tail space" of this buffer. + // Use GetSpan when possible for better efficiency. + public Memory GetMemory(int sizeHint = 0) + { + // TODO: implement + throw new NotImplementedException(); + } + + // provides access to the "tail space" of this buffer. + public unsafe Span GetSpan(int sizeHint = 0) { GrpcPreconditions.CheckArgument(sizeHint >= 0); - if (tailSpaceLen.ToUInt64() < (ulong) sizeHint) + if (tailSpaceLen < sizeHint) { // TODO: should we ignore the hint sometimes when // available tail space is close enough to the sizeHint? AdjustTailSpace(sizeHint); } - return new Span(tailSpacePtr.ToPointer(), (int) tailSpaceLen.ToUInt64()); + return new Span(tailSpacePtr.ToPointer(), tailSpaceLen); } public void Complete() @@ -81,8 +91,17 @@ namespace Grpc.Core.Internal AdjustTailSpace(0); } + // resets the data contained by this slice buffer + public void Reset() + { + // deletes all the data in the slice buffer + tailSpacePtr = IntPtr.Zero; + tailSpaceLen = 0; + Native.grpcsharp_slice_buffer_reset_and_unref(this); + } + // copies the content of the slice buffer to a newly allocated byte array - // due to its overhead, this method should only be used for testing. + // Note that this method has a relatively high overhead and should maily be used for testing. public byte[] ToByteArray() { ulong sliceCount = Native.grpcsharp_slice_buffer_slice_count(this).ToUInt64(); @@ -111,9 +130,8 @@ namespace Grpc.Core.Internal private void AdjustTailSpace(int requestedSize) { GrpcPreconditions.CheckArgument(requestedSize >= 0); - var requestedTailSpaceLen = new UIntPtr((ulong) requestedSize); - tailSpacePtr = Native.grpcsharp_slice_buffer_adjust_tail_space(this, tailSpaceLen, requestedTailSpaceLen); - tailSpaceLen = requestedTailSpaceLen; + tailSpacePtr = Native.grpcsharp_slice_buffer_adjust_tail_space(this, new UIntPtr((ulong) tailSpaceLen), new UIntPtr((ulong) requestedSize)); + tailSpaceLen = requestedSize; } protected override bool ReleaseHandle() { diff --git a/src/csharp/ext/grpc_csharp_ext.c b/src/csharp/ext/grpc_csharp_ext.c index 82fa81abf7d..78027d270dc 100644 --- a/src/csharp/ext/grpc_csharp_ext.c +++ b/src/csharp/ext/grpc_csharp_ext.c @@ -1195,6 +1195,11 @@ grpcsharp_slice_buffer_create() { return slice_buffer; } +GPR_EXPORT void GPR_CALLTYPE +grpcsharp_slice_buffer_reset_and_unref(grpc_slice_buffer* buffer) { + grpc_slice_buffer_reset_and_unref(buffer); +} + GPR_EXPORT void GPR_CALLTYPE grpcsharp_slice_buffer_destroy(grpc_slice_buffer* buffer) { grpc_slice_buffer_destroy(buffer); @@ -1214,8 +1219,6 @@ grpcsharp_slice_buffer_slice_peek(grpc_slice_buffer* buffer, size_t index, size_ *slice_data_ptr = GRPC_SLICE_START_PTR(*slice_ptr); } - - GPR_EXPORT void* GPR_CALLTYPE grpcsharp_slice_buffer_adjust_tail_space(grpc_slice_buffer* buffer, size_t available_tail_space, size_t requested_tail_space) { diff --git a/src/csharp/unitypackage/unitypackage_skeleton/Plugins/Grpc.Core/runtimes/grpc_csharp_ext_dummy_stubs.c b/src/csharp/unitypackage/unitypackage_skeleton/Plugins/Grpc.Core/runtimes/grpc_csharp_ext_dummy_stubs.c index 9007d7f3ad0..58a7c58e91a 100644 --- a/src/csharp/unitypackage/unitypackage_skeleton/Plugins/Grpc.Core/runtimes/grpc_csharp_ext_dummy_stubs.c +++ b/src/csharp/unitypackage/unitypackage_skeleton/Plugins/Grpc.Core/runtimes/grpc_csharp_ext_dummy_stubs.c @@ -390,6 +390,10 @@ void grpcsharp_slice_buffer_slice_peek() { fprintf(stderr, "Should never reach here"); abort(); } +void grpcsharp_slice_buffer_reset_and_unref() { + fprintf(stderr, "Should never reach here"); + abort(); +} void grpcsharp_slice_buffer_destroy() { fprintf(stderr, "Should never reach here"); abort(); diff --git a/templates/src/csharp/Grpc.Core/Internal/native_methods.include b/templates/src/csharp/Grpc.Core/Internal/native_methods.include index 4f5f61d332c..f2b3a165a3b 100644 --- a/templates/src/csharp/Grpc.Core/Internal/native_methods.include +++ b/templates/src/csharp/Grpc.Core/Internal/native_methods.include @@ -92,6 +92,7 @@ native_method_signatures = [ 'IntPtr grpcsharp_slice_buffer_adjust_tail_space(SliceBufferSafeHandle sliceBuffer, UIntPtr availableTailSpace, UIntPtr requestedTailSpace)', 'UIntPtr grpcsharp_slice_buffer_slice_count(SliceBufferSafeHandle sliceBuffer)', 'void grpcsharp_slice_buffer_slice_peek(SliceBufferSafeHandle sliceBuffer, UIntPtr index, out UIntPtr sliceLen, out IntPtr sliceDataPtr)', + 'void grpcsharp_slice_buffer_reset_and_unref(SliceBufferSafeHandle sliceBuffer)', 'void grpcsharp_slice_buffer_destroy(IntPtr sliceBuffer)', 'Timespec gprsharp_now(ClockType clockType)', 'Timespec gprsharp_inf_future(ClockType clockType)', From aaddd42c00647a0fea619d7dfda090c038f6d7fa Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 31 Jul 2019 17:29:04 -0700 Subject: [PATCH 09/28] add serializationScope and refactoring for efficiency --- .../ContextualMarshallerTest.cs | 2 + src/csharp/Grpc.Core/Internal/AsyncCall.cs | 15 +++++-- .../Grpc.Core/Internal/AsyncCallBase.cs | 44 ++++++++----------- .../Grpc.Core/Internal/AsyncCallServer.cs | 39 ++++++++-------- .../Internal/DefaultSerializationContext.cs | 40 ++++++++++------- 5 files changed, 78 insertions(+), 62 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/ContextualMarshallerTest.cs b/src/csharp/Grpc.Core.Tests/ContextualMarshallerTest.cs index c3aee726f26..99f92e12628 100644 --- a/src/csharp/Grpc.Core.Tests/ContextualMarshallerTest.cs +++ b/src/csharp/Grpc.Core.Tests/ContextualMarshallerTest.cs @@ -52,6 +52,8 @@ namespace Grpc.Core.Tests } if (str == "SERIALIZE_TO_NULL") { + // TODO: test for not calling complete Complete() (that resulted in null payload before...) + // TODO: test for calling Complete(null byte array) return; } var bytes = System.Text.Encoding.UTF8.GetBytes(str); diff --git a/src/csharp/Grpc.Core/Internal/AsyncCall.cs b/src/csharp/Grpc.Core/Internal/AsyncCall.cs index aefa58a0cee..4111c5347ce 100644 --- a/src/csharp/Grpc.Core/Internal/AsyncCall.cs +++ b/src/csharp/Grpc.Core/Internal/AsyncCall.cs @@ -95,10 +95,10 @@ namespace Grpc.Core.Internal readingDone = true; } - var payload = UnsafeSerialize(msg); - + using (var serializationScope = DefaultSerializationContext.GetInitializedThreadLocalScope()) using (var metadataArray = MetadataArraySafeHandle.Create(details.Options.Headers)) { + var payload = UnsafeSerialize(msg, serializationScope.Context); // do before metadata array? var ctx = details.Channel.Environment.BatchContextPool.Lease(); try { @@ -160,11 +160,14 @@ namespace Grpc.Core.Internal halfcloseRequested = true; readingDone = true; - var payload = UnsafeSerialize(msg); + //var payload = UnsafeSerialize(msg); unaryResponseTcs = new TaskCompletionSource(); + using (var serializationScope = DefaultSerializationContext.GetInitializedThreadLocalScope()) using (var metadataArray = MetadataArraySafeHandle.Create(details.Options.Headers)) { + var payload = UnsafeSerialize(msg, serializationScope.Context); // do before metadata array? + call.StartUnary(UnaryResponseClientCallback, payload, GetWriteFlagsForCall(), metadataArray, details.Options.Flags); callStartedOk = true; } @@ -235,11 +238,15 @@ namespace Grpc.Core.Internal halfcloseRequested = true; - var payload = UnsafeSerialize(msg); + //var payload = UnsafeSerialize(msg); streamingResponseCallFinishedTcs = new TaskCompletionSource(); + + using (var serializationScope = DefaultSerializationContext.GetInitializedThreadLocalScope()) using (var metadataArray = MetadataArraySafeHandle.Create(details.Options.Headers)) { + var payload = UnsafeSerialize(msg, serializationScope.Context); // do before metadata array? + call.StartServerStreaming(ReceivedStatusOnClientCallback, payload, GetWriteFlagsForCall(), metadataArray, details.Options.Flags); callStartedOk = true; } diff --git a/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs b/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs index 07f9ecf23e9..bd4b0d81832 100644 --- a/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs +++ b/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs @@ -115,23 +115,25 @@ namespace Grpc.Core.Internal /// protected Task SendMessageInternalAsync(TWrite msg, WriteFlags writeFlags) { - var payload = UnsafeSerialize(msg); - - lock (myLock) + using (var serializationScope = DefaultSerializationContext.GetInitializedThreadLocalScope()) { - GrpcPreconditions.CheckState(started); - var earlyResult = CheckSendAllowedOrEarlyResult(); - if (earlyResult != null) + var payload = UnsafeSerialize(msg, serializationScope.Context); // do before metadata array? + lock (myLock) { - return earlyResult; - } + GrpcPreconditions.CheckState(started); + var earlyResult = CheckSendAllowedOrEarlyResult(); + if (earlyResult != null) + { + return earlyResult; + } - call.StartSendMessage(SendCompletionCallback, payload, writeFlags, !initialMetadataSent); + call.StartSendMessage(SendCompletionCallback, payload, writeFlags, !initialMetadataSent); - initialMetadataSent = true; - streamingWritesCounter++; - streamingWriteTcs = new TaskCompletionSource(); - return streamingWriteTcs.Task; + initialMetadataSent = true; + streamingWritesCounter++; + streamingWriteTcs = new TaskCompletionSource(); + return streamingWriteTcs.Task; + } } } @@ -213,19 +215,11 @@ namespace Grpc.Core.Internal /// protected abstract Task CheckSendAllowedOrEarlyResult(); - protected SliceBufferSafeHandle UnsafeSerialize(TWrite msg) + // runs the serializer, propagating any exceptions being thrown without modifying them + protected SliceBufferSafeHandle UnsafeSerialize(TWrite msg, DefaultSerializationContext context) { - DefaultSerializationContext context = null; - try - { - context = DefaultSerializationContext.GetInitializedThreadLocal(); - serializer(msg, context); - return context.GetPayload(); - } - finally - { - context?.Reset(); - } + serializer(msg, context); + return context.GetPayload(); } protected Exception TryDeserialize(IBufferReader reader, out TRead msg) diff --git a/src/csharp/Grpc.Core/Internal/AsyncCallServer.cs b/src/csharp/Grpc.Core/Internal/AsyncCallServer.cs index e1c3a215422..e0bb41e15dc 100644 --- a/src/csharp/Grpc.Core/Internal/AsyncCallServer.cs +++ b/src/csharp/Grpc.Core/Internal/AsyncCallServer.cs @@ -129,28 +129,31 @@ namespace Grpc.Core.Internal /// public Task SendStatusFromServerAsync(Status status, Metadata trailers, ResponseWithFlags? optionalWrite) { - var payload = optionalWrite.HasValue ? UnsafeSerialize(optionalWrite.Value.Response) : null; - var writeFlags = optionalWrite.HasValue ? optionalWrite.Value.WriteFlags : default(WriteFlags); - - lock (myLock) + using (var serializationScope = DefaultSerializationContext.GetInitializedThreadLocalScope()) { - GrpcPreconditions.CheckState(started); - GrpcPreconditions.CheckState(!disposed); - GrpcPreconditions.CheckState(!halfcloseRequested, "Can only send status from server once."); + var payload = optionalWrite.HasValue ? UnsafeSerialize(optionalWrite.Value.Response, serializationScope.Context) : null; + var writeFlags = optionalWrite.HasValue ? optionalWrite.Value.WriteFlags : default(WriteFlags); - using (var metadataArray = MetadataArraySafeHandle.Create(trailers)) - { - call.StartSendStatusFromServer(SendStatusFromServerCompletionCallback, status, metadataArray, !initialMetadataSent, - payload, writeFlags); - } - halfcloseRequested = true; - initialMetadataSent = true; - sendStatusFromServerTcs = new TaskCompletionSource(); - if (optionalWrite.HasValue) + lock (myLock) { - streamingWritesCounter++; + GrpcPreconditions.CheckState(started); + GrpcPreconditions.CheckState(!disposed); + GrpcPreconditions.CheckState(!halfcloseRequested, "Can only send status from server once."); + + using (var metadataArray = MetadataArraySafeHandle.Create(trailers)) + { + call.StartSendStatusFromServer(SendStatusFromServerCompletionCallback, status, metadataArray, !initialMetadataSent, + payload, writeFlags); + } + halfcloseRequested = true; + initialMetadataSent = true; + sendStatusFromServerTcs = new TaskCompletionSource(); + if (optionalWrite.HasValue) + { + streamingWritesCounter++; + } + return sendStatusFromServerTcs.Task; } - return sendStatusFromServerTcs.Task; } } diff --git a/src/csharp/Grpc.Core/Internal/DefaultSerializationContext.cs b/src/csharp/Grpc.Core/Internal/DefaultSerializationContext.cs index db5e78a608d..6bf9da56264 100644 --- a/src/csharp/Grpc.Core/Internal/DefaultSerializationContext.cs +++ b/src/csharp/Grpc.Core/Internal/DefaultSerializationContext.cs @@ -29,8 +29,7 @@ namespace Grpc.Core.Internal new ThreadLocal(() => new DefaultSerializationContext(), false); bool isComplete; - //byte[] payload; - SliceBufferSafeHandle sliceBuffer; + SliceBufferSafeHandle sliceBuffer = SliceBufferSafeHandle.Create(); public DefaultSerializationContext() { @@ -42,12 +41,10 @@ namespace Grpc.Core.Internal GrpcPreconditions.CheckState(!isComplete); this.isComplete = true; - GetBufferWriter(); var destSpan = sliceBuffer.GetSpan(payload.Length); payload.AsSpan().CopyTo(destSpan); sliceBuffer.Advance(payload.Length); sliceBuffer.Complete(); - //this.payload = payload; } /// @@ -55,11 +52,6 @@ namespace Grpc.Core.Internal /// public override IBufferWriter GetBufferWriter() { - if (sliceBuffer == null) - { - // TODO: avoid allocation.. - sliceBuffer = SliceBufferSafeHandle.Create(); - } return sliceBuffer; } @@ -81,17 +73,35 @@ namespace Grpc.Core.Internal public void Reset() { this.isComplete = false; - //this.payload = null; - this.sliceBuffer = null; // reset instead... + this.sliceBuffer.Reset(); } - public static DefaultSerializationContext GetInitializedThreadLocal() + // Get a cached thread local instance of deserialization context + // and wrap it in a disposable struct that allows easy resetting + // via "using" statement. + public static UsageScope GetInitializedThreadLocalScope() { var instance = threadLocalInstance.Value; - instance.Reset(); - return instance; + return new UsageScope(instance); } - + public struct UsageScope : IDisposable + { + readonly DefaultSerializationContext context; + + public UsageScope(DefaultSerializationContext context) + { + this.context = context; + } + + public DefaultSerializationContext Context => context; + + // TODO: add Serialize method... + + public void Dispose() + { + context.Reset(); + } + } } } From 74dcdbb3c39619c1a725dc9a7e62c2cb5b62eda4 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 7 Aug 2019 12:57:38 -0700 Subject: [PATCH 10/28] fix adjust_tail_space --- src/csharp/ext/grpc_csharp_ext.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/csharp/ext/grpc_csharp_ext.c b/src/csharp/ext/grpc_csharp_ext.c index 78027d270dc..89391e67e15 100644 --- a/src/csharp/ext/grpc_csharp_ext.c +++ b/src/csharp/ext/grpc_csharp_ext.c @@ -1223,32 +1223,30 @@ GPR_EXPORT void* GPR_CALLTYPE grpcsharp_slice_buffer_adjust_tail_space(grpc_slice_buffer* buffer, size_t available_tail_space, size_t requested_tail_space) { - if (available_tail_space == 0 && requested_tail_space == 0) - { - return NULL; + if (available_tail_space == requested_tail_space) { + // nothing to do } - // TODO: what if available_tail_space == requested_tail_space == 0 - - if (available_tail_space >= requested_tail_space) + else if (available_tail_space >= requested_tail_space) { - // TODO: should this be allowed at all? - grpc_slice_buffer garbage; - grpc_slice_buffer_trim_end(buffer, available_tail_space - requested_tail_space, &garbage); - grpc_slice_buffer_reset_and_unref(&garbage); + grpc_slice_buffer_trim_end(buffer, available_tail_space - requested_tail_space, NULL); } else { if (available_tail_space > 0) { - grpc_slice_buffer garbage; - grpc_slice_buffer_trim_end(buffer, available_tail_space, &garbage); - grpc_slice_buffer_reset_and_unref(&garbage); + grpc_slice_buffer_trim_end(buffer, available_tail_space, NULL); } grpc_slice new_slice = grpc_slice_malloc(requested_tail_space); - grpc_slice_buffer_add(buffer, new_slice); + // TODO: this always adds as a new slice entry into the sb, but it doesn't have the problem of + // sometimes splitting the continguous new_slice across two different slices (like grpc_slice_buffer_add would) + grpc_slice_buffer_add_indexed(buffer, new_slice); } + if (buffer->count == 0) + { + return NULL; + } grpc_slice* last_slice = &(buffer->slices[buffer->count - 1]); return GRPC_SLICE_END_PTR(*last_slice) - requested_tail_space; } From f32bd8b091669f2ce1056d2ddbae694dbb54cb18 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 7 Aug 2019 12:59:57 -0700 Subject: [PATCH 11/28] add default serialization context test --- .../DefaultSerializationContextTest.cs | 144 ++++++++++++++++++ src/csharp/tests.json | 1 + 2 files changed, 145 insertions(+) create mode 100644 src/csharp/Grpc.Core.Tests/Internal/DefaultSerializationContextTest.cs diff --git a/src/csharp/Grpc.Core.Tests/Internal/DefaultSerializationContextTest.cs b/src/csharp/Grpc.Core.Tests/Internal/DefaultSerializationContextTest.cs new file mode 100644 index 00000000000..4d09d80bb72 --- /dev/null +++ b/src/csharp/Grpc.Core.Tests/Internal/DefaultSerializationContextTest.cs @@ -0,0 +1,144 @@ +#region Copyright notice and license + +// Copyright 2019 The gRPC Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#endregion + +using System; +using System.Buffers; +using System.Collections.Generic; +using System.Runtime.InteropServices; +using Grpc.Core; +using Grpc.Core.Internal; +using Grpc.Core.Utils; +using NUnit.Framework; + +namespace Grpc.Core.Internal.Tests +{ + public class DefaultSerializationContextTest + { + [TestCase] + public void CompleteAllowedOnlyOnce() + { + var context = new DefaultSerializationContext(); + var buffer = GetTestBuffer(10); + + context.Complete(buffer); + Assert.Throws(typeof(InvalidOperationException), () => context.Complete(buffer)); + Assert.Throws(typeof(InvalidOperationException), () => context.Complete()); + } + + [TestCase] + public void CompleteAllowedOnlyOnce2() + { + var context = new DefaultSerializationContext(); + + context.Complete(); + Assert.Throws(typeof(InvalidOperationException), () => context.Complete(GetTestBuffer(10))); + Assert.Throws(typeof(InvalidOperationException), () => context.Complete()); + } + + [TestCase(0)] + [TestCase(1)] + [TestCase(10)] + [TestCase(100)] + [TestCase(1000)] + public void ByteArrayPayload(int payloadSize) + { + var context = new DefaultSerializationContext(); + var origPayload = GetTestBuffer(payloadSize); + + context.Complete(origPayload); + + var nativePayload = context.GetPayload().ToByteArray(); + CollectionAssert.AreEqual(origPayload, nativePayload); + } + + [TestCase(0)] + [TestCase(1)] + [TestCase(10)] + [TestCase(100)] + [TestCase(1000)] + public void BufferWriter_OneSegment(int payloadSize) + { + var context = new DefaultSerializationContext(); + var origPayload = GetTestBuffer(payloadSize); + + var bufferWriter = context.GetBufferWriter(); + origPayload.AsSpan().CopyTo(bufferWriter.GetSpan(payloadSize)); + bufferWriter.Advance(payloadSize); + // TODO: test that call to Complete() is required. + + var nativePayload = context.GetPayload().ToByteArray(); + CollectionAssert.AreEqual(origPayload, nativePayload); + } + + [TestCase(1, 4)] // small slice size tests grpc_slice with inline data + [TestCase(10, 4)] + [TestCase(100, 4)] + [TestCase(1000, 4)] + [TestCase(1, 64)] // larger slice size tests allocated grpc_slices + [TestCase(10, 64)] + [TestCase(1000, 50)] + [TestCase(1000, 64)] + public void BufferWriter_MultipleSegments(int payloadSize, int maxSliceSize) + { + var context = new DefaultSerializationContext(); + var origPayload = GetTestBuffer(payloadSize); + + var bufferWriter = context.GetBufferWriter(); + for (int offset = 0; offset < payloadSize; offset += maxSliceSize) + { + var sliceSize = Math.Min(maxSliceSize, payloadSize - offset); + // we allocate last slice as too big intentionally to test that shrinking works + var dest = bufferWriter.GetSpan(maxSliceSize); + + origPayload.AsSpan(offset, sliceSize).CopyTo(dest); + bufferWriter.Advance(sliceSize); + } + context.Complete(); + + var nativePayload = context.GetPayload().ToByteArray(); + CollectionAssert.AreEqual(origPayload, nativePayload); + + context.GetPayload().Dispose(); // TODO: do it better.. (use the scope...) + } + + // AdjustTailSpace(0) if previous tail size is 0.... + + // test that context.Complete() call is required... + + // BufferWriter.GetMemory... (add refs to the original memory?) + + // TODO: set payload and then get IBufferWriter should throw? + + // TODO: test Reset()... + + // TODO: destroy SliceBufferSafeHandles... (use usagescope...) + + + + + private byte[] GetTestBuffer(int length) + { + var testBuffer = new byte[length]; + for (int i = 0; i < testBuffer.Length; i++) + { + testBuffer[i] = (byte) i; + } + return testBuffer; + } + } +} diff --git a/src/csharp/tests.json b/src/csharp/tests.json index 2eb786e0983..75bda4c25ea 100644 --- a/src/csharp/tests.json +++ b/src/csharp/tests.json @@ -9,6 +9,7 @@ "Grpc.Core.Internal.Tests.CompletionQueueSafeHandleTest", "Grpc.Core.Internal.Tests.DefaultDeserializationContextTest", "Grpc.Core.Internal.Tests.DefaultObjectPoolTest", + "Grpc.Core.Internal.Tests.DefaultSerializationContextTest", "Grpc.Core.Internal.Tests.FakeBufferReaderManagerTest", "Grpc.Core.Internal.Tests.MetadataArraySafeHandleTest", "Grpc.Core.Internal.Tests.ReusableSliceBufferTest", From d57dec1c7dc19a245ddc74971781b53610bda4cc Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 9 Aug 2019 14:03:18 -0700 Subject: [PATCH 12/28] improve DefaultSerializationContextTest --- .../DefaultSerializationContextTest.cs | 151 ++++++++++++------ 1 file changed, 104 insertions(+), 47 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/Internal/DefaultSerializationContextTest.cs b/src/csharp/Grpc.Core.Tests/Internal/DefaultSerializationContextTest.cs index 4d09d80bb72..aa7631587be 100644 --- a/src/csharp/Grpc.Core.Tests/Internal/DefaultSerializationContextTest.cs +++ b/src/csharp/Grpc.Core.Tests/Internal/DefaultSerializationContextTest.cs @@ -17,9 +17,6 @@ #endregion using System; -using System.Buffers; -using System.Collections.Generic; -using System.Runtime.InteropServices; using Grpc.Core; using Grpc.Core.Internal; using Grpc.Core.Utils; @@ -32,22 +29,28 @@ namespace Grpc.Core.Internal.Tests [TestCase] public void CompleteAllowedOnlyOnce() { - var context = new DefaultSerializationContext(); - var buffer = GetTestBuffer(10); + using (var scope = NewDefaultSerializationContextScope()) + { + var context = scope.Context; + var buffer = GetTestBuffer(10); - context.Complete(buffer); - Assert.Throws(typeof(InvalidOperationException), () => context.Complete(buffer)); - Assert.Throws(typeof(InvalidOperationException), () => context.Complete()); + context.Complete(buffer); + Assert.Throws(typeof(InvalidOperationException), () => context.Complete(buffer)); + Assert.Throws(typeof(InvalidOperationException), () => context.Complete()); + } } [TestCase] public void CompleteAllowedOnlyOnce2() { - var context = new DefaultSerializationContext(); + using (var scope = NewDefaultSerializationContextScope()) + { + var context = scope.Context; - context.Complete(); - Assert.Throws(typeof(InvalidOperationException), () => context.Complete(GetTestBuffer(10))); - Assert.Throws(typeof(InvalidOperationException), () => context.Complete()); + context.Complete(); + Assert.Throws(typeof(InvalidOperationException), () => context.Complete(GetTestBuffer(10))); + Assert.Throws(typeof(InvalidOperationException), () => context.Complete()); + } } [TestCase(0)] @@ -57,13 +60,16 @@ namespace Grpc.Core.Internal.Tests [TestCase(1000)] public void ByteArrayPayload(int payloadSize) { - var context = new DefaultSerializationContext(); - var origPayload = GetTestBuffer(payloadSize); + using (var scope = NewDefaultSerializationContextScope()) + { + var context = scope.Context; + var origPayload = GetTestBuffer(payloadSize); - context.Complete(origPayload); + context.Complete(origPayload); - var nativePayload = context.GetPayload().ToByteArray(); - CollectionAssert.AreEqual(origPayload, nativePayload); + var nativePayload = context.GetPayload().ToByteArray(); + CollectionAssert.AreEqual(origPayload, nativePayload); + } } [TestCase(0)] @@ -73,16 +79,40 @@ namespace Grpc.Core.Internal.Tests [TestCase(1000)] public void BufferWriter_OneSegment(int payloadSize) { - var context = new DefaultSerializationContext(); - var origPayload = GetTestBuffer(payloadSize); + using (var scope = NewDefaultSerializationContextScope()) + { + var context = scope.Context; + var origPayload = GetTestBuffer(payloadSize); - var bufferWriter = context.GetBufferWriter(); - origPayload.AsSpan().CopyTo(bufferWriter.GetSpan(payloadSize)); - bufferWriter.Advance(payloadSize); - // TODO: test that call to Complete() is required. + var bufferWriter = context.GetBufferWriter(); + origPayload.AsSpan().CopyTo(bufferWriter.GetSpan(payloadSize)); + bufferWriter.Advance(payloadSize); + // TODO: test that call to Complete() is required. - var nativePayload = context.GetPayload().ToByteArray(); - CollectionAssert.AreEqual(origPayload, nativePayload); + var nativePayload = context.GetPayload().ToByteArray(); + CollectionAssert.AreEqual(origPayload, nativePayload); + } + } + + [TestCase(0)] + [TestCase(1)] + [TestCase(10)] + [TestCase(100)] + [TestCase(1000)] + public void BufferWriter_OneSegment_GetMemory(int payloadSize) + { + using (var scope = NewDefaultSerializationContextScope()) + { + var context = scope.Context; + var origPayload = GetTestBuffer(payloadSize); + + var bufferWriter = context.GetBufferWriter(); + origPayload.AsSpan().CopyTo(bufferWriter.GetMemory(payloadSize).Span); + bufferWriter.Advance(payloadSize); + + var nativePayload = context.GetPayload().ToByteArray(); + CollectionAssert.AreEqual(origPayload, nativePayload); + } } [TestCase(1, 4)] // small slice size tests grpc_slice with inline data @@ -95,41 +125,68 @@ namespace Grpc.Core.Internal.Tests [TestCase(1000, 64)] public void BufferWriter_MultipleSegments(int payloadSize, int maxSliceSize) { - var context = new DefaultSerializationContext(); - var origPayload = GetTestBuffer(payloadSize); - - var bufferWriter = context.GetBufferWriter(); - for (int offset = 0; offset < payloadSize; offset += maxSliceSize) + using (var scope = NewDefaultSerializationContextScope()) { - var sliceSize = Math.Min(maxSliceSize, payloadSize - offset); - // we allocate last slice as too big intentionally to test that shrinking works - var dest = bufferWriter.GetSpan(maxSliceSize); + var context = scope.Context; + var origPayload = GetTestBuffer(payloadSize); + + var bufferWriter = context.GetBufferWriter(); + for (int offset = 0; offset < payloadSize; offset += maxSliceSize) + { + var sliceSize = Math.Min(maxSliceSize, payloadSize - offset); + // we allocate last slice as too big intentionally to test that shrinking works + var dest = bufferWriter.GetSpan(maxSliceSize); + + origPayload.AsSpan(offset, sliceSize).CopyTo(dest); + bufferWriter.Advance(sliceSize); + } + context.Complete(); - origPayload.AsSpan(offset, sliceSize).CopyTo(dest); - bufferWriter.Advance(sliceSize); + var nativePayload = context.GetPayload().ToByteArray(); + CollectionAssert.AreEqual(origPayload, nativePayload); } - context.Complete(); - - var nativePayload = context.GetPayload().ToByteArray(); - CollectionAssert.AreEqual(origPayload, nativePayload); - - context.GetPayload().Dispose(); // TODO: do it better.. (use the scope...) } - // AdjustTailSpace(0) if previous tail size is 0.... + [TestCase] + public void ContextIsReusable() + { + using (var scope = NewDefaultSerializationContextScope()) + { + var context = scope.Context; + + var origPayload1 = GetTestBuffer(10); + context.Complete(origPayload1); + CollectionAssert.AreEqual(origPayload1, context.GetPayload().ToByteArray()); - // test that context.Complete() call is required... + context.Reset(); - // BufferWriter.GetMemory... (add refs to the original memory?) + var origPayload2 = GetTestBuffer(20); + + var bufferWriter = context.GetBufferWriter(); + origPayload2.AsSpan().CopyTo(bufferWriter.GetMemory(origPayload2.Length).Span); + bufferWriter.Advance(origPayload2.Length); + CollectionAssert.AreEqual(origPayload2, context.GetPayload().ToByteArray()); - // TODO: set payload and then get IBufferWriter should throw? + context.Reset(); + + // TODO: that's should be a null payload... + CollectionAssert.AreEqual(new byte[0], context.GetPayload().ToByteArray()); + } + } - // TODO: test Reset()... + //test ideas: - // TODO: destroy SliceBufferSafeHandles... (use usagescope...) + // test that context.Complete() call is required... + // set payload with Complete([]) and then get IBufferWriter should throw (because it's been completed already?) + // other ideas: + // AdjustTailSpace(0) if previous tail size is 0... (better for SliceBufferSafeHandle) + private DefaultSerializationContext.UsageScope NewDefaultSerializationContextScope() + { + return new DefaultSerializationContext.UsageScope(new DefaultSerializationContext()); + } private byte[] GetTestBuffer(int length) { From 389d759344a58e8ae2be85623905b11da0b8e969 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 9 Aug 2019 14:19:33 -0700 Subject: [PATCH 13/28] improve doc comments for SerializationContext --- src/csharp/Grpc.Core.Api/SerializationContext.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/csharp/Grpc.Core.Api/SerializationContext.cs b/src/csharp/Grpc.Core.Api/SerializationContext.cs index 79107040411..76f1951b68e 100644 --- a/src/csharp/Grpc.Core.Api/SerializationContext.cs +++ b/src/csharp/Grpc.Core.Api/SerializationContext.cs @@ -28,7 +28,7 @@ namespace Grpc.Core { /// /// Use the byte array as serialized form of current message and mark serialization process as complete. - /// Complete() can only be called once. By calling this method the caller gives up the ownership of the + /// Complete() can only be called once. By calling this method the caller gives up the ownership of the /// payload which must not be accessed afterwards. /// /// the serialized form of current message @@ -38,7 +38,8 @@ namespace Grpc.Core } /// - /// Expose serializer as buffer writer + /// Gets buffer writer that can be used to write the serialized data. Once serialization is finished, + /// Complete() needs to be called. /// public virtual IBufferWriter GetBufferWriter() { @@ -46,7 +47,7 @@ namespace Grpc.Core } /// - /// Complete the payload written so far. + /// Complete the payload written to the buffer writer. Complete() can only be called once. /// public virtual void Complete() { From 891dc61d8e238fa07e5323164bf00189bff42d3f Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 9 Aug 2019 14:28:29 -0700 Subject: [PATCH 14/28] move slice memory manager --- .../Grpc.Core/Internal/ReusableSliceBuffer.cs | 41 +-------- .../Grpc.Core/Internal/SliceMemoryManager.cs | 87 +++++++++++++++++++ 2 files changed, 88 insertions(+), 40 deletions(-) create mode 100644 src/csharp/Grpc.Core/Internal/SliceMemoryManager.cs diff --git a/src/csharp/Grpc.Core/Internal/ReusableSliceBuffer.cs b/src/csharp/Grpc.Core/Internal/ReusableSliceBuffer.cs index 078e59e9d1b..eed6ba4ae15 100644 --- a/src/csharp/Grpc.Core/Internal/ReusableSliceBuffer.cs +++ b/src/csharp/Grpc.Core/Internal/ReusableSliceBuffer.cs @@ -101,45 +101,6 @@ namespace Grpc.Core.Internal { Next = next; } - } - - // Allow creating instances of Memory from Slice. - // Represents a chunk of native memory, but doesn't manage its lifetime. - // Instances of this class are reuseable - they can be reset to point to a different memory chunk. - // That is important to make the instances cacheable (rather then creating new instances - // the old ones will be reused to reduce GC pressure). - private class SliceMemoryManager : MemoryManager - { - private Slice slice; - - public void Reset(Slice slice) - { - this.slice = slice; - } - - public void Reset() - { - Reset(new Slice(IntPtr.Zero, 0)); - } - - public override Span GetSpan() - { - return slice.ToSpanUnsafe(); - } - - public override MemoryHandle Pin(int elementIndex = 0) - { - throw new NotSupportedException(); - } - - public override void Unpin() - { - } - - protected override void Dispose(bool disposing) - { - // NOP - } - } + } } } diff --git a/src/csharp/Grpc.Core/Internal/SliceMemoryManager.cs b/src/csharp/Grpc.Core/Internal/SliceMemoryManager.cs new file mode 100644 index 00000000000..342d90df36c --- /dev/null +++ b/src/csharp/Grpc.Core/Internal/SliceMemoryManager.cs @@ -0,0 +1,87 @@ +#region Copyright notice and license + +// Copyright 2019 The gRPC Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#endregion + +using Grpc.Core.Utils; +using System; +using System.Threading; + +using System.Buffers; + +namespace Grpc.Core.Internal +{ + internal class ReusableSliceBuffer + { + public const int MaxCachedSegments = 1024; // ~4MB payload for 4K slices + + readonly SliceSegment[] cachedSegments = new SliceSegment[MaxCachedSegments]; + int populatedSegmentCount; + + public ReadOnlySequence PopulateFrom(IBufferReader bufferReader) + { + populatedSegmentCount = 0; + long offset = 0; + SliceSegment prevSegment = null; + while (bufferReader.TryGetNextSlice(out Slice slice)) + { + // Initialize cached segment if still null or just allocate a new segment if we already reached MaxCachedSegments + var current = populatedSegmentCount < cachedSegments.Length ? cachedSegments[populatedSegmentCount] : new SliceSegment(); + if (current == null) + { + current = cachedSegments[populatedSegmentCount] = new SliceSegment(); + } + + current.Reset(slice, offset); + prevSegment?.SetNext(current); + + populatedSegmentCount ++; + offset += slice.Length; + prevSegment = current; + } + + // Not necessary for ending the ReadOnlySequence, but for making sure we + // don't keep more than MaxCachedSegments alive. + prevSegment?.SetNext(null); + + if (populatedSegmentCount == 0) + { + return ReadOnlySequence.Empty; + } + + var firstSegment = cachedSegments[0]; + var lastSegment = prevSegment; + return new ReadOnlySequence(firstSegment, 0, lastSegment, lastSegment.Memory.Length); + } + + public void Invalidate() + { + if (populatedSegmentCount == 0) + { + return; + } + var segment = cachedSegments[0]; + while (segment != null) + { + segment.Reset(new Slice(IntPtr.Zero, 0), 0); + var nextSegment = (SliceSegment) segment.Next; + segment.SetNext(null); + segment = nextSegment; + } + populatedSegmentCount = 0; + } + } +} From fb5411fba9d91f73814b6c107a268c374ad58721 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 9 Aug 2019 14:30:28 -0700 Subject: [PATCH 15/28] support GetMemory() --- .../Grpc.Core/Internal/SliceBufferSafeHandle.cs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs b/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs index 76a97f4a0a2..0ce37317f1a 100644 --- a/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs +++ b/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs @@ -39,6 +39,8 @@ namespace Grpc.Core.Internal private IntPtr tailSpacePtr; private int tailSpaceLen; + private SliceMemoryManager memoryManagerLazy; + private SliceBufferSafeHandle() { } @@ -63,14 +65,20 @@ namespace Grpc.Core.Internal GrpcPreconditions.CheckArgument(tailSpaceLen >= count); tailSpaceLen = tailSpaceLen - count; tailSpacePtr += count; + memoryManagerLazy?.Reset(); } // provides access to the "tail space" of this buffer. // Use GetSpan when possible for better efficiency. public Memory GetMemory(int sizeHint = 0) { - // TODO: implement - throw new NotImplementedException(); + GetSpan(sizeHint); + if (memoryManagerLazy == null) + { + memoryManagerLazy = new SliceMemoryManager(); + } + memoryManagerLazy.Reset(new Slice(tailSpacePtr, tailSpaceLen)); + return memoryManagerLazy.Memory; } // provides access to the "tail space" of this buffer. @@ -97,6 +105,7 @@ namespace Grpc.Core.Internal // deletes all the data in the slice buffer tailSpacePtr = IntPtr.Zero; tailSpaceLen = 0; + memoryManagerLazy?.Reset(); Native.grpcsharp_slice_buffer_reset_and_unref(this); } From 66cd7cbb8fce6136ead7fd05fe79356c2cf072c9 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 9 Aug 2019 14:31:01 -0700 Subject: [PATCH 16/28] improve default serialization context tests --- .../DefaultSerializationContextTest.cs | 23 +++++++++++++------ .../Internal/DefaultSerializationContext.cs | 9 +++++--- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/Internal/DefaultSerializationContextTest.cs b/src/csharp/Grpc.Core.Tests/Internal/DefaultSerializationContextTest.cs index aa7631587be..fcb12ae6656 100644 --- a/src/csharp/Grpc.Core.Tests/Internal/DefaultSerializationContextTest.cs +++ b/src/csharp/Grpc.Core.Tests/Internal/DefaultSerializationContextTest.cs @@ -87,7 +87,7 @@ namespace Grpc.Core.Internal.Tests var bufferWriter = context.GetBufferWriter(); origPayload.AsSpan().CopyTo(bufferWriter.GetSpan(payloadSize)); bufferWriter.Advance(payloadSize); - // TODO: test that call to Complete() is required. + context.Complete(); var nativePayload = context.GetPayload().ToByteArray(); CollectionAssert.AreEqual(origPayload, nativePayload); @@ -109,6 +109,7 @@ namespace Grpc.Core.Internal.Tests var bufferWriter = context.GetBufferWriter(); origPayload.AsSpan().CopyTo(bufferWriter.GetMemory(payloadSize).Span); bufferWriter.Advance(payloadSize); + context.Complete(); var nativePayload = context.GetPayload().ToByteArray(); CollectionAssert.AreEqual(origPayload, nativePayload); @@ -154,6 +155,8 @@ namespace Grpc.Core.Internal.Tests { var context = scope.Context; + Assert.Throws(typeof(NullReferenceException), () => context.GetPayload()); + var origPayload1 = GetTestBuffer(10); context.Complete(origPayload1); CollectionAssert.AreEqual(origPayload1, context.GetPayload().ToByteArray()); @@ -165,20 +168,26 @@ namespace Grpc.Core.Internal.Tests var bufferWriter = context.GetBufferWriter(); origPayload2.AsSpan().CopyTo(bufferWriter.GetMemory(origPayload2.Length).Span); bufferWriter.Advance(origPayload2.Length); + context.Complete(); CollectionAssert.AreEqual(origPayload2, context.GetPayload().ToByteArray()); context.Reset(); - // TODO: that's should be a null payload... - CollectionAssert.AreEqual(new byte[0], context.GetPayload().ToByteArray()); + Assert.Throws(typeof(NullReferenceException), () => context.GetPayload()); } } - //test ideas: - - // test that context.Complete() call is required... + [TestCase] + public void GetBufferWriterThrowsForCompletedContext() + { + using (var scope = NewDefaultSerializationContextScope()) + { + var context = scope.Context; + context.Complete(GetTestBuffer(10)); - // set payload with Complete([]) and then get IBufferWriter should throw (because it's been completed already?) + Assert.Throws(typeof(InvalidOperationException), () => context.GetBufferWriter()); + } + } // other ideas: // AdjustTailSpace(0) if previous tail size is 0... (better for SliceBufferSafeHandle) diff --git a/src/csharp/Grpc.Core/Internal/DefaultSerializationContext.cs b/src/csharp/Grpc.Core/Internal/DefaultSerializationContext.cs index 6bf9da56264..4d45b5c684f 100644 --- a/src/csharp/Grpc.Core/Internal/DefaultSerializationContext.cs +++ b/src/csharp/Grpc.Core/Internal/DefaultSerializationContext.cs @@ -52,6 +52,7 @@ namespace Grpc.Core.Internal /// public override IBufferWriter GetBufferWriter() { + GrpcPreconditions.CheckState(!isComplete); return sliceBuffer; } @@ -67,6 +68,11 @@ namespace Grpc.Core.Internal internal SliceBufferSafeHandle GetPayload() { + if (!isComplete) + { + // mimic the legacy behavior when byte[] was used to represent the payload. + throw new NullReferenceException("No payload was set. Complete() needs to be called before payload can be used."); + } return sliceBuffer; } @@ -95,9 +101,6 @@ namespace Grpc.Core.Internal } public DefaultSerializationContext Context => context; - - // TODO: add Serialize method... - public void Dispose() { context.Reset(); From 532bdf7cd29f613fdfe2184b4c7035398def5353 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 9 Aug 2019 14:35:00 -0700 Subject: [PATCH 17/28] fixup SliceMemoryManager --- .../Grpc.Core/Internal/SliceMemoryManager.cs | 74 +++++++------------ 1 file changed, 26 insertions(+), 48 deletions(-) diff --git a/src/csharp/Grpc.Core/Internal/SliceMemoryManager.cs b/src/csharp/Grpc.Core/Internal/SliceMemoryManager.cs index 342d90df36c..dad2d96bd75 100644 --- a/src/csharp/Grpc.Core/Internal/SliceMemoryManager.cs +++ b/src/csharp/Grpc.Core/Internal/SliceMemoryManager.cs @@ -24,64 +24,42 @@ using System.Buffers; namespace Grpc.Core.Internal { - internal class ReusableSliceBuffer + // Allow creating instances of Memory from Slice. + // Represents a chunk of native memory, but doesn't manage its lifetime. + // Instances of this class are reuseable - they can be reset to point to a different memory chunk. + // That is important to make the instances cacheable (rather then creating new instances + // the old ones will be reused to reduce GC pressure). + internal class SliceMemoryManager : MemoryManager { - public const int MaxCachedSegments = 1024; // ~4MB payload for 4K slices + private Slice slice; - readonly SliceSegment[] cachedSegments = new SliceSegment[MaxCachedSegments]; - int populatedSegmentCount; - - public ReadOnlySequence PopulateFrom(IBufferReader bufferReader) + public void Reset(Slice slice) { - populatedSegmentCount = 0; - long offset = 0; - SliceSegment prevSegment = null; - while (bufferReader.TryGetNextSlice(out Slice slice)) - { - // Initialize cached segment if still null or just allocate a new segment if we already reached MaxCachedSegments - var current = populatedSegmentCount < cachedSegments.Length ? cachedSegments[populatedSegmentCount] : new SliceSegment(); - if (current == null) - { - current = cachedSegments[populatedSegmentCount] = new SliceSegment(); - } - - current.Reset(slice, offset); - prevSegment?.SetNext(current); + this.slice = slice; + } - populatedSegmentCount ++; - offset += slice.Length; - prevSegment = current; - } + public void Reset() + { + Reset(new Slice(IntPtr.Zero, 0)); + } - // Not necessary for ending the ReadOnlySequence, but for making sure we - // don't keep more than MaxCachedSegments alive. - prevSegment?.SetNext(null); + public override Span GetSpan() + { + return slice.ToSpanUnsafe(); + } - if (populatedSegmentCount == 0) - { - return ReadOnlySequence.Empty; - } + public override MemoryHandle Pin(int elementIndex = 0) + { + throw new NotSupportedException(); + } - var firstSegment = cachedSegments[0]; - var lastSegment = prevSegment; - return new ReadOnlySequence(firstSegment, 0, lastSegment, lastSegment.Memory.Length); + public override void Unpin() + { } - public void Invalidate() + protected override void Dispose(bool disposing) { - if (populatedSegmentCount == 0) - { - return; - } - var segment = cachedSegments[0]; - while (segment != null) - { - segment.Reset(new Slice(IntPtr.Zero, 0), 0); - var nextSegment = (SliceSegment) segment.Next; - segment.SetNext(null); - segment = nextSegment; - } - populatedSegmentCount = 0; + // NOP } } } From 79c9aa081d7ee9cc6368edc66be8428fe54f0dc6 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 9 Aug 2019 14:38:45 -0700 Subject: [PATCH 18/28] add slice buffer safe handle tests --- .../Internal/SliceBufferSafeHandleTest.cs | 38 +++++++++++++++++++ src/csharp/tests.json | 1 + 2 files changed, 39 insertions(+) create mode 100644 src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs diff --git a/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs b/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs new file mode 100644 index 00000000000..563187948d2 --- /dev/null +++ b/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs @@ -0,0 +1,38 @@ +#region Copyright notice and license + +// Copyright 2019 The gRPC Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#endregion + +using System; +using Grpc.Core; +using Grpc.Core.Internal; +using Grpc.Core.Utils; +using NUnit.Framework; + +namespace Grpc.Core.Internal.Tests +{ + public class SliceBufferSafeHandleTest + { + [TestCase] + public void BasicTest() + { + + } + + // other ideas: + // AdjustTailSpace(0) if previous tail size is 0... (better for SliceBufferSafeHandle) + } +} diff --git a/src/csharp/tests.json b/src/csharp/tests.json index 75bda4c25ea..e18266ab9d7 100644 --- a/src/csharp/tests.json +++ b/src/csharp/tests.json @@ -13,6 +13,7 @@ "Grpc.Core.Internal.Tests.FakeBufferReaderManagerTest", "Grpc.Core.Internal.Tests.MetadataArraySafeHandleTest", "Grpc.Core.Internal.Tests.ReusableSliceBufferTest", + "Grpc.Core.Internal.Tests.SliceBufferSafeHandleTest", "Grpc.Core.Internal.Tests.SliceTest", "Grpc.Core.Internal.Tests.TimespecTest", "Grpc.Core.Internal.Tests.WellKnownStringsTest", From ec14872fc283ac36b6898ab1f9d0fad758d6329e Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 9 Aug 2019 14:48:36 -0700 Subject: [PATCH 19/28] address TODO" --- src/csharp/Grpc.Core.Tests/ContextualMarshallerTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/ContextualMarshallerTest.cs b/src/csharp/Grpc.Core.Tests/ContextualMarshallerTest.cs index 99f92e12628..dbceb27baf9 100644 --- a/src/csharp/Grpc.Core.Tests/ContextualMarshallerTest.cs +++ b/src/csharp/Grpc.Core.Tests/ContextualMarshallerTest.cs @@ -52,8 +52,8 @@ namespace Grpc.Core.Tests } if (str == "SERIALIZE_TO_NULL") { - // TODO: test for not calling complete Complete() (that resulted in null payload before...) - // TODO: test for calling Complete(null byte array) + // for contextual marshaller, serializing to null payload corresponds + // to not calling the Complete() method in the serializer. return; } var bytes = System.Text.Encoding.UTF8.GetBytes(str); From ded29be883e888081174a9be88de4ce2618303af Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 9 Aug 2019 15:43:57 -0700 Subject: [PATCH 20/28] slice buffer improvements --- .../Internal/SliceBufferSafeHandleTest.cs | 95 ++++++++++++++++++- .../Internal/SliceBufferSafeHandle.cs | 33 +++++-- 2 files changed, 115 insertions(+), 13 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs b/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs index 563187948d2..61c817dafc9 100644 --- a/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs +++ b/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs @@ -27,12 +27,99 @@ namespace Grpc.Core.Internal.Tests public class SliceBufferSafeHandleTest { [TestCase] - public void BasicTest() + public void Complete_EmptyBuffer() { - + using (var sliceBuffer = SliceBufferSafeHandle.Create()) + { + sliceBuffer.Complete(); + CollectionAssert.AreEqual(new byte[0], sliceBuffer.ToByteArray()); + } } - // other ideas: - // AdjustTailSpace(0) if previous tail size is 0... (better for SliceBufferSafeHandle) + [TestCase] + public void Complete_TailSizeZero() + { + using (var sliceBuffer = SliceBufferSafeHandle.Create()) + { + var origPayload = GetTestBuffer(10); + origPayload.AsSpan().CopyTo(sliceBuffer.GetSpan(origPayload.Length)); + sliceBuffer.Advance(origPayload.Length); + // call complete where tail space size == 0 + sliceBuffer.Complete(); + CollectionAssert.AreEqual(origPayload, sliceBuffer.ToByteArray()); + } + } + + [TestCase] + public void Complete_TruncateTailSpace() + { + using (var sliceBuffer = SliceBufferSafeHandle.Create()) + { + var origPayload = GetTestBuffer(10); + var dest = sliceBuffer.GetSpan(origPayload.Length + 10); + origPayload.AsSpan().CopyTo(dest); + sliceBuffer.Advance(origPayload.Length); + // call complete where tail space needs to be truncated + sliceBuffer.Complete(); + CollectionAssert.AreEqual(origPayload, sliceBuffer.ToByteArray()); + } + } + + [TestCase] + public void SliceBufferIsReusable() + { + using (var sliceBuffer = SliceBufferSafeHandle.Create()) + { + var origPayload = GetTestBuffer(10); + origPayload.AsSpan().CopyTo(sliceBuffer.GetSpan(origPayload.Length)); + sliceBuffer.Advance(origPayload.Length); + sliceBuffer.Complete(); + CollectionAssert.AreEqual(origPayload, sliceBuffer.ToByteArray()); + + sliceBuffer.Reset(); + + var origPayload2 = GetTestBuffer(20); + origPayload2.AsSpan().CopyTo(sliceBuffer.GetSpan(origPayload2.Length)); + sliceBuffer.Advance(origPayload2.Length); + sliceBuffer.Complete(); + CollectionAssert.AreEqual(origPayload2, sliceBuffer.ToByteArray()); + + sliceBuffer.Reset(); + + CollectionAssert.AreEqual(new byte[0], sliceBuffer.ToByteArray()); + } + } + + [TestCase] + public void SliceBuffer_SizeHintZero() + { + using (var sliceBuffer = SliceBufferSafeHandle.Create()) + { + var destSpan = sliceBuffer.GetSpan(0); + Assert.IsTrue(destSpan.Length > 0); // some non-zero size memory is made available + + sliceBuffer.Reset(); + + var destMemory = sliceBuffer.GetMemory(0); + Assert.IsTrue(destMemory.Length > 0); + } + } + + // Advance() - multiple chunks... + + // other TODOS: + + // -- provide a null instance of SliceBufferSafeHandle + //-- order of UnsafeSerialize in AsyncCall methods... + + private byte[] GetTestBuffer(int length) + { + var testBuffer = new byte[length]; + for (int i = 0; i < testBuffer.Length; i++) + { + testBuffer[i] = (byte) i; + } + return testBuffer; + } } } diff --git a/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs b/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs index 0ce37317f1a..77c14f1d9fb 100644 --- a/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs +++ b/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs @@ -31,6 +31,7 @@ namespace Grpc.Core.Internal /// internal class SliceBufferSafeHandle : SafeHandleZeroIsInvalid, IBufferWriter { + const int DefaultTailSpaceSize = 4096; // default buffer to allocate if no size hint is provided static readonly NativeMethods Native = NativeMethods.Get(); static readonly ILogger Logger = GrpcEnvironment.Logger.ForType(); @@ -72,7 +73,7 @@ namespace Grpc.Core.Internal // Use GetSpan when possible for better efficiency. public Memory GetMemory(int sizeHint = 0) { - GetSpan(sizeHint); + EnsureBufferSpace(sizeHint); if (memoryManagerLazy == null) { memoryManagerLazy = new SliceMemoryManager(); @@ -84,13 +85,7 @@ namespace Grpc.Core.Internal // provides access to the "tail space" of this buffer. public unsafe Span GetSpan(int sizeHint = 0) { - GrpcPreconditions.CheckArgument(sizeHint >= 0); - if (tailSpaceLen < sizeHint) - { - // TODO: should we ignore the hint sometimes when - // available tail space is close enough to the sizeHint? - AdjustTailSpace(sizeHint); - } + EnsureBufferSpace(sizeHint); return new Span(tailSpacePtr.ToPointer(), tailSpaceLen); } @@ -135,7 +130,27 @@ namespace Grpc.Core.Internal return result; } - // Gets data of server_rpc_new completion. + private void EnsureBufferSpace(int sizeHint) + { + GrpcPreconditions.CheckArgument(sizeHint >= 0); + if (sizeHint == 0) + { + // if no hint is provided, keep the available space within some "reasonable" boundaries. + // This is quite a naive approach which could use some fine-tuning, but currently in most case we know + // the required buffer size in advance anyway, so this approach seems good enough for now. + if (tailSpaceLen < DefaultTailSpaceSize /2 ) + { + AdjustTailSpace(DefaultTailSpaceSize); + } + } + else if (tailSpaceLen < sizeHint) + { + // if hint is provided, always make sure we provide at least that much space + AdjustTailSpace(sizeHint); + } + } + + // make sure there's exactly requestedSize bytes of continguous buffer space at the end of this slice buffer private void AdjustTailSpace(int requestedSize) { GrpcPreconditions.CheckArgument(requestedSize >= 0); From 127d69383abc0c442625038be6d57522b35f20c6 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 9 Aug 2019 16:00:41 -0700 Subject: [PATCH 21/28] better tests --- .../Internal/SliceBufferSafeHandleTest.cs | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs b/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs index 61c817dafc9..5feb9a711da 100644 --- a/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs +++ b/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs @@ -105,10 +105,52 @@ namespace Grpc.Core.Internal.Tests } } - // Advance() - multiple chunks... + [TestCase(0)] + [TestCase(1000)] + public void SliceBuffer_BigPayload(int sizeHint) + { + using (var sliceBuffer = SliceBufferSafeHandle.Create()) + { + var bigPayload = GetTestBuffer(4 * 1024 * 1024); + + int offset = 0; + while (offset < bigPayload.Length) + { + var destSpan = sliceBuffer.GetSpan(sizeHint); + int copySize = Math.Min(destSpan.Length, bigPayload.Length - offset); + bigPayload.AsSpan(offset, copySize).CopyTo(destSpan); + sliceBuffer.Advance(copySize); + offset += copySize; + } + + sliceBuffer.Complete(); + CollectionAssert.AreEqual(bigPayload, sliceBuffer.ToByteArray()); + } + } - // other TODOS: + [TestCase] + public void SliceBuffer_NegativeSizeHint() + { + using (var sliceBuffer = SliceBufferSafeHandle.Create()) + { + Assert.Throws(typeof(ArgumentException), () => sliceBuffer.GetSpan(-1)); + Assert.Throws(typeof(ArgumentException), () => sliceBuffer.GetMemory(-1)); + } + } + + [TestCase] + public void SliceBuffer_AdvanceBadArg() + { + using (var sliceBuffer = SliceBufferSafeHandle.Create()) + { + int size = 10; + var destSpan = sliceBuffer.GetSpan(size); + Assert.Throws(typeof(ArgumentException), () => sliceBuffer.Advance(size + 1)); + Assert.Throws(typeof(ArgumentException), () => sliceBuffer.Advance(-1)); + } + } + // TODO: other TODOs // -- provide a null instance of SliceBufferSafeHandle //-- order of UnsafeSerialize in AsyncCall methods... From ec351c1ac2329be334adb93a46f04c2a80a1ec39 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 12 Aug 2019 17:00:12 -0700 Subject: [PATCH 22/28] reorder UnsafeSerialize --- .../Internal/SliceBufferSafeHandleTest.cs | 1 - src/csharp/Grpc.Core/Internal/AsyncCall.cs | 31 +++++++++---------- .../Grpc.Core/Internal/AsyncCallBase.cs | 2 +- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs b/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs index 5feb9a711da..89d5f187a22 100644 --- a/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs +++ b/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs @@ -152,7 +152,6 @@ namespace Grpc.Core.Internal.Tests // TODO: other TODOs // -- provide a null instance of SliceBufferSafeHandle - //-- order of UnsafeSerialize in AsyncCall methods... private byte[] GetTestBuffer(int length) { diff --git a/src/csharp/Grpc.Core/Internal/AsyncCall.cs b/src/csharp/Grpc.Core/Internal/AsyncCall.cs index 4111c5347ce..830a1f4edc6 100644 --- a/src/csharp/Grpc.Core/Internal/AsyncCall.cs +++ b/src/csharp/Grpc.Core/Internal/AsyncCall.cs @@ -160,16 +160,15 @@ namespace Grpc.Core.Internal halfcloseRequested = true; readingDone = true; - //var payload = UnsafeSerialize(msg); - - unaryResponseTcs = new TaskCompletionSource(); using (var serializationScope = DefaultSerializationContext.GetInitializedThreadLocalScope()) - using (var metadataArray = MetadataArraySafeHandle.Create(details.Options.Headers)) { - var payload = UnsafeSerialize(msg, serializationScope.Context); // do before metadata array? - - call.StartUnary(UnaryResponseClientCallback, payload, GetWriteFlagsForCall(), metadataArray, details.Options.Flags); - callStartedOk = true; + var payload = UnsafeSerialize(msg, serializationScope.Context); + unaryResponseTcs = new TaskCompletionSource(); + using (var metadataArray = MetadataArraySafeHandle.Create(details.Options.Headers)) + { + call.StartUnary(UnaryResponseClientCallback, payload, GetWriteFlagsForCall(), metadataArray, details.Options.Flags); + callStartedOk = true; + } } return unaryResponseTcs.Task; @@ -238,17 +237,15 @@ namespace Grpc.Core.Internal halfcloseRequested = true; - //var payload = UnsafeSerialize(msg); - - streamingResponseCallFinishedTcs = new TaskCompletionSource(); - using (var serializationScope = DefaultSerializationContext.GetInitializedThreadLocalScope()) - using (var metadataArray = MetadataArraySafeHandle.Create(details.Options.Headers)) { - var payload = UnsafeSerialize(msg, serializationScope.Context); // do before metadata array? - - call.StartServerStreaming(ReceivedStatusOnClientCallback, payload, GetWriteFlagsForCall(), metadataArray, details.Options.Flags); - callStartedOk = true; + var payload = UnsafeSerialize(msg, serializationScope.Context); + streamingResponseCallFinishedTcs = new TaskCompletionSource(); + using (var metadataArray = MetadataArraySafeHandle.Create(details.Options.Headers)) + { + call.StartServerStreaming(ReceivedStatusOnClientCallback, payload, GetWriteFlagsForCall(), metadataArray, details.Options.Flags); + callStartedOk = true; + } } call.StartReceiveInitialMetadata(ReceivedResponseHeadersCallback); } diff --git a/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs b/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs index bd4b0d81832..252fe114506 100644 --- a/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs +++ b/src/csharp/Grpc.Core/Internal/AsyncCallBase.cs @@ -117,7 +117,7 @@ namespace Grpc.Core.Internal { using (var serializationScope = DefaultSerializationContext.GetInitializedThreadLocalScope()) { - var payload = UnsafeSerialize(msg, serializationScope.Context); // do before metadata array? + var payload = UnsafeSerialize(msg, serializationScope.Context); lock (myLock) { GrpcPreconditions.CheckState(started); From 36dcea650dc333ca0ba19942f01742f4bec602f9 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 12 Aug 2019 17:03:33 -0700 Subject: [PATCH 23/28] address a TODO --- src/csharp/ext/grpc_csharp_ext.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/csharp/ext/grpc_csharp_ext.c b/src/csharp/ext/grpc_csharp_ext.c index 89391e67e15..9ffc5591d8b 100644 --- a/src/csharp/ext/grpc_csharp_ext.c +++ b/src/csharp/ext/grpc_csharp_ext.c @@ -1238,8 +1238,9 @@ grpcsharp_slice_buffer_adjust_tail_space(grpc_slice_buffer* buffer, size_t avail } grpc_slice new_slice = grpc_slice_malloc(requested_tail_space); - // TODO: this always adds as a new slice entry into the sb, but it doesn't have the problem of - // sometimes splitting the continguous new_slice across two different slices (like grpc_slice_buffer_add would) + // grpc_slice_buffer_add_indexed always adds as a new slice entry into the sb (which is suboptimal in some cases) + // but it doesn't have the problem of sometimes splitting the continguous new_slice across two different slices + // (like grpc_slice_buffer_add would) grpc_slice_buffer_add_indexed(buffer, new_slice); } From dd5515870747ecdc5cfd92398aa02fd47b667132 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 12 Aug 2019 17:20:27 -0700 Subject: [PATCH 24/28] address TODO in SendMessageBenchmark --- .../Grpc.Microbenchmarks/SendMessageBenchmark.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/csharp/Grpc.Microbenchmarks/SendMessageBenchmark.cs b/src/csharp/Grpc.Microbenchmarks/SendMessageBenchmark.cs index 76c59ca579c..f850a0a006f 100644 --- a/src/csharp/Grpc.Microbenchmarks/SendMessageBenchmark.cs +++ b/src/csharp/Grpc.Microbenchmarks/SendMessageBenchmark.cs @@ -50,18 +50,21 @@ namespace Grpc.Microbenchmarks var call = CreateFakeCall(cq); var sendCompletionCallback = new NopSendCompletionCallback(); - var payload = SliceBufferSafeHandle.Create(); - payload.GetSpan(PayloadSize); - payload.Advance(PayloadSize); + var sliceBuffer = SliceBufferSafeHandle.Create(); var writeFlags = default(WriteFlags); for (int i = 0; i < Iterations; i++) { - // TODO: sending for the first time actually steals the slices... - call.StartSendMessage(sendCompletionCallback, payload, writeFlags, false); + // SendMessage steals the slices from the slice buffer, so we need to repopulate in each iteration. + sliceBuffer.Reset(); + sliceBuffer.GetSpan(PayloadSize); + sliceBuffer.Advance(PayloadSize); + + call.StartSendMessage(sendCompletionCallback, sliceBuffer, writeFlags, false); var callback = completionRegistry.Extract(completionRegistry.LastRegisteredKey); callback.OnComplete(true); } + sliceBuffer.Dispose(); cq.Dispose(); } From 9ae5c5df245a2ed683de6028e3aa55bb88723269 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 12 Aug 2019 17:35:00 -0700 Subject: [PATCH 25/28] address a TODO --- .../Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs | 3 --- src/csharp/Grpc.Core/Internal/AsyncCallServer.cs | 2 +- src/csharp/Grpc.Core/Internal/CallSafeHandle.cs | 7 ------- src/csharp/Grpc.Microbenchmarks/Utf8Encode.cs | 2 +- 4 files changed, 2 insertions(+), 12 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs b/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs index 89d5f187a22..76387478312 100644 --- a/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs +++ b/src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs @@ -150,9 +150,6 @@ namespace Grpc.Core.Internal.Tests } } - // TODO: other TODOs - // -- provide a null instance of SliceBufferSafeHandle - private byte[] GetTestBuffer(int length) { var testBuffer = new byte[length]; diff --git a/src/csharp/Grpc.Core/Internal/AsyncCallServer.cs b/src/csharp/Grpc.Core/Internal/AsyncCallServer.cs index e0bb41e15dc..58bc40b0ace 100644 --- a/src/csharp/Grpc.Core/Internal/AsyncCallServer.cs +++ b/src/csharp/Grpc.Core/Internal/AsyncCallServer.cs @@ -131,7 +131,7 @@ namespace Grpc.Core.Internal { using (var serializationScope = DefaultSerializationContext.GetInitializedThreadLocalScope()) { - var payload = optionalWrite.HasValue ? UnsafeSerialize(optionalWrite.Value.Response, serializationScope.Context) : null; + var payload = optionalWrite.HasValue ? UnsafeSerialize(optionalWrite.Value.Response, serializationScope.Context) : SliceBufferSafeHandle.NullInstance; var writeFlags = optionalWrite.HasValue ? optionalWrite.Value.WriteFlags : default(WriteFlags); lock (myLock) diff --git a/src/csharp/Grpc.Core/Internal/CallSafeHandle.cs b/src/csharp/Grpc.Core/Internal/CallSafeHandle.cs index cfcab5dc692..7fadc1d8b47 100644 --- a/src/csharp/Grpc.Core/Internal/CallSafeHandle.cs +++ b/src/csharp/Grpc.Core/Internal/CallSafeHandle.cs @@ -131,13 +131,6 @@ namespace Grpc.Core.Internal public void StartSendStatusFromServer(ISendStatusFromServerCompletionCallback callback, Status status, MetadataArraySafeHandle metadataArray, bool sendEmptyInitialMetadata, SliceBufferSafeHandle optionalPayload, WriteFlags writeFlags) { - // TODO: optionalPayload == null -> pass null SliceBufferSafeHandle - // do this properly - if (optionalPayload == null) - { - optionalPayload = SliceBufferSafeHandle.NullInstance; - } - using (completionQueue.NewScope()) { var ctx = completionQueue.CompletionRegistry.RegisterBatchCompletion(CompletionHandler_ISendStatusFromServerCompletionCallback, callback); diff --git a/src/csharp/Grpc.Microbenchmarks/Utf8Encode.cs b/src/csharp/Grpc.Microbenchmarks/Utf8Encode.cs index e5bac1967c9..8aa2e96832a 100644 --- a/src/csharp/Grpc.Microbenchmarks/Utf8Encode.cs +++ b/src/csharp/Grpc.Microbenchmarks/Utf8Encode.cs @@ -117,7 +117,7 @@ namespace Grpc.Microbenchmarks { for (int i = 0; i < Iterations; i++) { - call.StartSendStatusFromServer(this, status, metadata, false, null, WriteFlags.NoCompress); + call.StartSendStatusFromServer(this, status, metadata, false, SliceBufferSafeHandle.NullInstance, WriteFlags.NoCompress); } } From 2f5464754516f2922e1a1d2711b0a90f3dc25070 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 12 Aug 2019 20:38:49 -0400 Subject: [PATCH 26/28] clang format --- src/csharp/ext/grpc_csharp_ext.c | 93 ++++++++++++++++---------------- 1 file changed, 46 insertions(+), 47 deletions(-) diff --git a/src/csharp/ext/grpc_csharp_ext.c b/src/csharp/ext/grpc_csharp_ext.c index 9ffc5591d8b..2360c9e1d5f 100644 --- a/src/csharp/ext/grpc_csharp_ext.c +++ b/src/csharp/ext/grpc_csharp_ext.c @@ -41,9 +41,10 @@ #define GPR_CALLTYPE #endif -static grpc_byte_buffer* grpcsharp_create_byte_buffer_from_stolen_slices(grpc_slice_buffer* slice_buffer) { +static grpc_byte_buffer* grpcsharp_create_byte_buffer_from_stolen_slices( + grpc_slice_buffer* slice_buffer) { grpc_byte_buffer* bb = - (grpc_byte_buffer*)gpr_malloc(sizeof(grpc_byte_buffer)); + (grpc_byte_buffer*)gpr_malloc(sizeof(grpc_byte_buffer)); memset(bb, 0, sizeof(grpc_byte_buffer)); bb->type = GRPC_BB_RAW; bb->data.raw.compression = GRPC_COMPRESS_NONE; @@ -587,8 +588,8 @@ static grpc_call_error grpcsharp_call_start_batch(grpc_call* call, } GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_unary( - grpc_call* call, grpcsharp_batch_context* ctx, grpc_slice_buffer* send_buffer, - uint32_t write_flags, + grpc_call* call, grpcsharp_batch_context* ctx, + grpc_slice_buffer* send_buffer, uint32_t write_flags, grpc_metadata_array* initial_metadata, uint32_t initial_metadata_flags) { /* TODO: don't use magic number */ grpc_op ops[6]; @@ -603,7 +604,8 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_unary( ops[0].reserved = NULL; ops[1].op = GRPC_OP_SEND_MESSAGE; - ctx->send_message = grpcsharp_create_byte_buffer_from_stolen_slices(send_buffer); + ctx->send_message = + grpcsharp_create_byte_buffer_from_stolen_slices(send_buffer); ops[1].data.send_message.send_message = ctx->send_message; ops[1].flags = write_flags; ops[1].reserved = NULL; @@ -640,8 +642,8 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_unary( /* Only for testing. Shortcircuits the unary call logic and only echoes the message as if it was received from the server */ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_test_call_start_unary_echo( - grpc_call* call, grpcsharp_batch_context* ctx, grpc_slice_buffer* send_buffer, - uint32_t write_flags, + grpc_call* call, grpcsharp_batch_context* ctx, + grpc_slice_buffer* send_buffer, uint32_t write_flags, grpc_metadata_array* initial_metadata, uint32_t initial_metadata_flags) { // prepare as if we were performing a normal RPC. grpc_byte_buffer* send_message = @@ -698,8 +700,8 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_client_streaming( } GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_server_streaming( - grpc_call* call, grpcsharp_batch_context* ctx, grpc_slice_buffer* send_buffer, - uint32_t write_flags, + grpc_call* call, grpcsharp_batch_context* ctx, + grpc_slice_buffer* send_buffer, uint32_t write_flags, grpc_metadata_array* initial_metadata, uint32_t initial_metadata_flags) { /* TODO: don't use magic number */ grpc_op ops[4]; @@ -714,7 +716,8 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_server_streaming( ops[0].reserved = NULL; ops[1].op = GRPC_OP_SEND_MESSAGE; - ctx->send_message = grpcsharp_create_byte_buffer_from_stolen_slices(send_buffer); + ctx->send_message = + grpcsharp_create_byte_buffer_from_stolen_slices(send_buffer); ops[1].data.send_message.send_message = ctx->send_message; ops[1].flags = write_flags; ops[1].reserved = NULL; @@ -781,15 +784,16 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_recv_initial_metadata( } GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_send_message( - grpc_call* call, grpcsharp_batch_context* ctx, grpc_slice_buffer* send_buffer, - uint32_t write_flags, + grpc_call* call, grpcsharp_batch_context* ctx, + grpc_slice_buffer* send_buffer, uint32_t write_flags, int32_t send_empty_initial_metadata) { /* TODO: don't use magic number */ grpc_op ops[2]; memset(ops, 0, sizeof(ops)); size_t nops = send_empty_initial_metadata ? 2 : 1; ops[0].op = GRPC_OP_SEND_MESSAGE; - ctx->send_message = grpcsharp_create_byte_buffer_from_stolen_slices(send_buffer); + ctx->send_message = + grpcsharp_create_byte_buffer_from_stolen_slices(send_buffer); ops[0].data.send_message.send_message = ctx->send_message; ops[0].flags = write_flags; ops[0].reserved = NULL; @@ -816,8 +820,7 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_send_status_from_server( grpc_call* call, grpcsharp_batch_context* ctx, grpc_status_code status_code, const char* status_details, size_t status_details_len, grpc_metadata_array* trailing_metadata, int32_t send_empty_initial_metadata, - grpc_slice_buffer* optional_send_buffer, - uint32_t write_flags) { + grpc_slice_buffer* optional_send_buffer, uint32_t write_flags) { /* TODO: don't use magic number */ grpc_op ops[3]; memset(ops, 0, sizeof(ops)); @@ -1188,9 +1191,9 @@ GPR_EXPORT void GPR_CALLTYPE grpcsharp_redirect_log(grpcsharp_log_func func) { typedef void(GPR_CALLTYPE* test_callback_funcptr)(int32_t success); /* Slice buffer functionality */ -GPR_EXPORT grpc_slice_buffer* GPR_CALLTYPE -grpcsharp_slice_buffer_create() { - grpc_slice_buffer* slice_buffer = (grpc_slice_buffer*)gpr_malloc(sizeof(grpc_slice_buffer)); +GPR_EXPORT grpc_slice_buffer* GPR_CALLTYPE grpcsharp_slice_buffer_create() { + grpc_slice_buffer* slice_buffer = + (grpc_slice_buffer*)gpr_malloc(sizeof(grpc_slice_buffer)); grpc_slice_buffer_init(slice_buffer); return slice_buffer; } @@ -1212,44 +1215,40 @@ grpcsharp_slice_buffer_slice_count(grpc_slice_buffer* buffer) { } GPR_EXPORT void GPR_CALLTYPE -grpcsharp_slice_buffer_slice_peek(grpc_slice_buffer* buffer, size_t index, size_t* slice_len, uint8_t** slice_data_ptr) { +grpcsharp_slice_buffer_slice_peek(grpc_slice_buffer* buffer, size_t index, + size_t* slice_len, uint8_t** slice_data_ptr) { GPR_ASSERT(buffer->count > index); grpc_slice* slice_ptr = &buffer->slices[index]; *slice_len = GRPC_SLICE_LENGTH(*slice_ptr); *slice_data_ptr = GRPC_SLICE_START_PTR(*slice_ptr); } -GPR_EXPORT void* GPR_CALLTYPE -grpcsharp_slice_buffer_adjust_tail_space(grpc_slice_buffer* buffer, size_t available_tail_space, +GPR_EXPORT void* GPR_CALLTYPE grpcsharp_slice_buffer_adjust_tail_space( + grpc_slice_buffer* buffer, size_t available_tail_space, size_t requested_tail_space) { - - if (available_tail_space == requested_tail_space) { - // nothing to do - } - else if (available_tail_space >= requested_tail_space) - { - grpc_slice_buffer_trim_end(buffer, available_tail_space - requested_tail_space, NULL); + if (available_tail_space == requested_tail_space) { + // nothing to do + } else if (available_tail_space >= requested_tail_space) { + grpc_slice_buffer_trim_end( + buffer, available_tail_space - requested_tail_space, NULL); + } else { + if (available_tail_space > 0) { + grpc_slice_buffer_trim_end(buffer, available_tail_space, NULL); } - else - { - if (available_tail_space > 0) - { - grpc_slice_buffer_trim_end(buffer, available_tail_space, NULL); - } - grpc_slice new_slice = grpc_slice_malloc(requested_tail_space); - // grpc_slice_buffer_add_indexed always adds as a new slice entry into the sb (which is suboptimal in some cases) - // but it doesn't have the problem of sometimes splitting the continguous new_slice across two different slices - // (like grpc_slice_buffer_add would) - grpc_slice_buffer_add_indexed(buffer, new_slice); - } - - if (buffer->count == 0) - { - return NULL; - } - grpc_slice* last_slice = &(buffer->slices[buffer->count - 1]); - return GRPC_SLICE_END_PTR(*last_slice) - requested_tail_space; + grpc_slice new_slice = grpc_slice_malloc(requested_tail_space); + // grpc_slice_buffer_add_indexed always adds as a new slice entry into the + // sb (which is suboptimal in some cases) but it doesn't have the problem of + // sometimes splitting the continguous new_slice across two different slices + // (like grpc_slice_buffer_add would) + grpc_slice_buffer_add_indexed(buffer, new_slice); + } + + if (buffer->count == 0) { + return NULL; + } + grpc_slice* last_slice = &(buffer->slices[buffer->count - 1]); + return GRPC_SLICE_END_PTR(*last_slice) - requested_tail_space; } /* Version info */ From cb6d3a0623bd19510610c07f7d114b33dc603793 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 29 Aug 2019 14:52:19 +0200 Subject: [PATCH 27/28] address review feedback --- src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs | 2 +- src/csharp/ext/grpc_csharp_ext.c | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs b/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs index 77c14f1d9fb..9cbdce1cfe3 100644 --- a/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs +++ b/src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs @@ -138,7 +138,7 @@ namespace Grpc.Core.Internal // if no hint is provided, keep the available space within some "reasonable" boundaries. // This is quite a naive approach which could use some fine-tuning, but currently in most case we know // the required buffer size in advance anyway, so this approach seems good enough for now. - if (tailSpaceLen < DefaultTailSpaceSize /2 ) + if (tailSpaceLen < DefaultTailSpaceSize / 2) { AdjustTailSpace(DefaultTailSpaceSize); } diff --git a/src/csharp/ext/grpc_csharp_ext.c b/src/csharp/ext/grpc_csharp_ext.c index 2360c9e1d5f..d9bf85e02c7 100644 --- a/src/csharp/ext/grpc_csharp_ext.c +++ b/src/csharp/ext/grpc_csharp_ext.c @@ -44,8 +44,7 @@ static grpc_byte_buffer* grpcsharp_create_byte_buffer_from_stolen_slices( grpc_slice_buffer* slice_buffer) { grpc_byte_buffer* bb = - (grpc_byte_buffer*)gpr_malloc(sizeof(grpc_byte_buffer)); - memset(bb, 0, sizeof(grpc_byte_buffer)); + (grpc_byte_buffer*)gpr_zalloc(sizeof(grpc_byte_buffer)); bb->type = GRPC_BB_RAW; bb->data.raw.compression = GRPC_COMPRESS_NONE; grpc_slice_buffer_init(&bb->data.raw.slice_buffer); From b876b35457121878345de26a1542d24cbb2d56a4 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 5 Sep 2019 13:48:31 +0200 Subject: [PATCH 28/28] address review comments --- src/csharp/Grpc.Core.Api/SerializationContext.cs | 4 ++-- .../Internal/DefaultSerializationContextTest.cs | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/csharp/Grpc.Core.Api/SerializationContext.cs b/src/csharp/Grpc.Core.Api/SerializationContext.cs index 76f1951b68e..59e14c12e3b 100644 --- a/src/csharp/Grpc.Core.Api/SerializationContext.cs +++ b/src/csharp/Grpc.Core.Api/SerializationContext.cs @@ -28,7 +28,7 @@ namespace Grpc.Core { /// /// Use the byte array as serialized form of current message and mark serialization process as complete. - /// Complete() can only be called once. By calling this method the caller gives up the ownership of the + /// Complete(byte[]) can only be called once. By calling this method the caller gives up the ownership of the /// payload which must not be accessed afterwards. /// /// the serialized form of current message @@ -47,7 +47,7 @@ namespace Grpc.Core } /// - /// Complete the payload written to the buffer writer. Complete() can only be called once. + /// Complete the payload written to the buffer writer. Complete() can only be called once. /// public virtual void Complete() { diff --git a/src/csharp/Grpc.Core.Tests/Internal/DefaultSerializationContextTest.cs b/src/csharp/Grpc.Core.Tests/Internal/DefaultSerializationContextTest.cs index fcb12ae6656..061230d8ca4 100644 --- a/src/csharp/Grpc.Core.Tests/Internal/DefaultSerializationContextTest.cs +++ b/src/csharp/Grpc.Core.Tests/Internal/DefaultSerializationContextTest.cs @@ -189,9 +189,6 @@ namespace Grpc.Core.Internal.Tests } } - // other ideas: - // AdjustTailSpace(0) if previous tail size is 0... (better for SliceBufferSafeHandle) - private DefaultSerializationContext.UsageScope NewDefaultSerializationContextScope() { return new DefaultSerializationContext.UsageScope(new DefaultSerializationContext());