You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by GitBox <gi...@apache.org> on 2020/04/01 10:01:29 UTC
[GitHub] [tinkerpop] FlorianHockmann opened a new pull request #1273:
TINKERPOP-2349 Migrate JSON serialization to System.Text.Json in Gremlin.NET
FlorianHockmann opened a new pull request #1273: TINKERPOP-2349 Migrate JSON serialization to System.Text.Json in Gremlin.NET
URL: https://github.com/apache/tinkerpop/pull/1273
https://issues.apache.org/jira/browse/TINKERPOP-2349
.NET Core 3.0 is added as a target framework as it already includes support for `System.Text.Json` and therefore doesn't need the NuGet package.
Results from a simple benchmark with BenchmarkDotNet showed that this improves the performance of the deserialization roughly by a factor of 2. The deserialization now also allocates far less memory, by a factor of ~7 which also results in less garbage collections. The performance of the serialization however stayed basically the same. It got even slightly slower by ~4% in this benchmark, but it also
allocates a bit less memory (-12%). This seems acceptable in my opinion, given that it is only a very small increase, especially compared to the differences for the deserialization and because it allocates less
memory now.
Output from BenchmarkDotNet:
Before:
| Method | Mean | Error | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---------------- |---------:|---------:|---------:|-------:|-------:|------:|----------:|
| Serialization | 22.41 us | 0.616 us | 0.659 us | 1.5564 | - | - | 9.72 KB |
| Deserialization | 40.50 us | 1.075 us | 1.150 us | 6.3477 | 0.6104 | - | 39.06 KB |
After:
| Method | Mean | Error | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|---------------- |---------:|---------:|---------:|-------:|------:|------:|----------:|
| Serialization | 23.25 us | 0.103 us | 0.091 us | 1.3733 | - | - | 8.58 KB |
| Deserialization | 25.89 us | 0.583 us | 0.545 us | 0.9155 | - | - | 5.74 KB |
Another advantage of using System.Text.Json is that it supports [`Span<T>`](https://docs.microsoft.com/en-us/archive/msdn-magazine/2018/january/csharp-all-about-span-exploring-a-new-net-mainstay) so we could make use of that in the future to reduce memory allocations in the driver even more.
VOTE +1
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] FlorianHockmann merged pull request #1273:
TINKERPOP-2349 Migrate JSON serialization to System.Text.Json in Gremlin.NET
Posted by GitBox <gi...@apache.org>.
FlorianHockmann merged pull request #1273: TINKERPOP-2349 Migrate JSON serialization to System.Text.Json in Gremlin.NET
URL: https://github.com/apache/tinkerpop/pull/1273
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] FlorianHockmann commented on a change in pull request
#1273: TINKERPOP-2349 Migrate JSON serialization to System.Text.Json in
Gremlin.NET
Posted by GitBox <gi...@apache.org>.
FlorianHockmann commented on a change in pull request #1273: TINKERPOP-2349 Migrate JSON serialization to System.Text.Json in Gremlin.NET
URL: https://github.com/apache/tinkerpop/pull/1273#discussion_r401566157
##########
File path: gremlin-dotnet/glv/Gremlin.Net.csproj.template
##########
@@ -19,7 +19,7 @@ limitations under the License.
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup Label="Build">
- <TargetFramework>netstandard2.0</TargetFramework>
+ <TargetFrameworks>netstandard2.0;netcoreapp3.0</TargetFrameworks>
Review comment:
I wasn't sure here whether it makes sense or not. I thought that doing it like this is necessary to prevent the SDK from installing the NuGet package for `System.Text.Json` on .NET Core >= 3.0 as it's already included there. Or is that already detected automatically?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] jorgebay commented on a change in pull request #1273:
TINKERPOP-2349 Migrate JSON serialization to System.Text.Json in Gremlin.NET
Posted by GitBox <gi...@apache.org>.
jorgebay commented on a change in pull request #1273: TINKERPOP-2349 Migrate JSON serialization to System.Text.Json in Gremlin.NET
URL: https://github.com/apache/tinkerpop/pull/1273#discussion_r401598867
##########
File path: gremlin-dotnet/glv/Gremlin.Net.csproj.template
##########
@@ -19,7 +19,7 @@ limitations under the License.
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup Label="Build">
- <TargetFramework>netstandard2.0</TargetFramework>
+ <TargetFrameworks>netstandard2.0;netcoreapp3.0</TargetFrameworks>
Review comment:
It will be resolved automatically for `netcoreapp3.0` users.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] jorgebay commented on a change in pull request #1273:
TINKERPOP-2349 Migrate JSON serialization to System.Text.Json in Gremlin.NET
Posted by GitBox <gi...@apache.org>.
jorgebay commented on a change in pull request #1273: TINKERPOP-2349 Migrate JSON serialization to System.Text.Json in Gremlin.NET
URL: https://github.com/apache/tinkerpop/pull/1273#discussion_r401525494
##########
File path: gremlin-dotnet/glv/Gremlin.Net.csproj.template
##########
@@ -19,7 +19,7 @@ limitations under the License.
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup Label="Build">
- <TargetFramework>netstandard2.0</TargetFramework>
+ <TargetFrameworks>netstandard2.0;netcoreapp3.0</TargetFrameworks>
Review comment:
I think there's no need to add `netcoreapp3.0` as a target for the core project, given that `netcoreapp3.0` implements `netstandard2.0`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] FlorianHockmann commented on a change in pull request
#1273: TINKERPOP-2349 Migrate JSON serialization to System.Text.Json in
Gremlin.NET
Posted by GitBox <gi...@apache.org>.
FlorianHockmann commented on a change in pull request #1273: TINKERPOP-2349 Migrate JSON serialization to System.Text.Json in Gremlin.NET
URL: https://github.com/apache/tinkerpop/pull/1273#discussion_r401601278
##########
File path: gremlin-dotnet/glv/Gremlin.Net.csproj.template
##########
@@ -19,7 +19,7 @@ limitations under the License.
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup Label="Build">
- <TargetFramework>netstandard2.0</TargetFramework>
+ <TargetFrameworks>netstandard2.0;netcoreapp3.0</TargetFrameworks>
Review comment:
Good to know. Then I'll remove this target again.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services