[jira] [Commented] (THRIFT-4476) Typecasting problem on list items
[
https://issues.apache.org/jira/browse/THRIFT-4476?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16403844#comment-16403844
]
ASF GitHub Bot commented on THRIFT-4476:
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1496#discussion_r175276394
--- Diff: build/appveyor/MSVC-appveyor-build.bat ---
@@ -39,7 +39,7 @@ CD "%BUILDDIR%" || EXIT /B
-DWITH_PYTHON=%WITH_PYTHON% ^
-DWITH_%THREADMODEL%THREADS=ON ^
-DWITH_SHARED_LIB=OFF ^
--DWITH_STATIC_LIB=ON|| EXIT /B
+-DWITH_STATIC_LIB=ON ^|| EXIT /B
--- End diff --
The carat addition looks wrong to me; it belongs on the end of a string, so
in this case I think it does nothing at all?
> Typecasting problem on list items
> -
>
> Key: THRIFT-4476
> URL: https://issues.apache.org/jira/browse/THRIFT-4476
> Project: Thrift
> Issue Type: Bug
> Components: Java - Compiler
>Affects Versions: 0.11.0
>Reporter: Ozan Can Altiok
>Priority: Major
>
> I was trying to add the following into a thrift interface file.
> {{const list timeCoefficients = [24, 60, 60, 1000, 1000, 1000]}}
> With the definition given above the {{.thrift}} file compiled properly.
> However, I noticed that in Python, the items in {{timeCoefficients}} are
> considered to be integer although the list has been defined to include items
> of {{double}} type. Then I modified the definition as given below to make
> sure all the items are of type {{double}}.
> {{const list timeCoefficients = [24.0, 60.0, 60.0, 1000.0, 1000.0,
> 1000.0]}}
> After the change, I ran into the following error during compilation.
> {{[ERROR] .../TimeCoefficients.java:[402,48] error: no suitable method found
> for add(int)}}
> {{[ERROR] method Collection.add(Double) is not applicable}}
> {{[ERROR] (argument mismatch; int cannot be converted to Double)}}
> {{[ERROR] method List.add(Double) is not applicable}}
> {{[ERROR] (argument mismatch; int cannot be converted to Double)}}
> Next, I changed the line as follows and the compilation became successful.
> {{const list timeCoefficients = [24.1, 60.1, 60.1, 1000.1, 1000.1,
> 1000.1]}}
> When I reviewed the generated Java source files, I discovered that
> {{const list timeCoefficients = [24, 60, 60, 1000, 1000, 1000]}}
> compiles to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add((double)24);}}
> {{ timeCoefficients.add((double)60);}}
> {{ timeCoefficients.add((double)60);}}
> {{ timeCoefficients.add((double)1000);}}
> {{ timeCoefficients.add((double)1000);}}
> {{ timeCoefficients.add((double)1000);}}
> {{}}}
> whilst
> {{const list timeCoefficients = [24.0, 60.0, 60.0, 1000.0, 1000.0,
> 1000.0]}}
> compiles to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add(24);}}
> {{ timeCoefficients.add(60);}}
> {{ timeCoefficients.add(60);}}
> {{ timeCoefficients.add(1000);}}
> {{ timeCoefficients.add(1000);}}
> {{ timeCoefficients.add(1000);}}
> {{}}}
> which leads to an error.
> When I modified this line as follows
> {{const list timeCoefficients = [24.1, 60.1, 60.1, 1000.1, 1000.1,
> 1000.1]}}
> this line compiled to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add(24.1);}}
> {{ timeCoefficients.add(60.1);}}
> {{ timeCoefficients.add(60.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{}}}
> My guess is that, even if I put {{.0}} at the end of each numeric constant,
> on the Java side, Thrift compiler considers them to be {{double}} and does no
> typecasts. However, the {{.0}} s are getting lost somewhere and the items
> become integers in the generated Java code. Thus, when it comes to populating
> the array, Java cannot succeed. {{Double}} s cannot be unboxed to integer and
> Java thinks {{int}} and {{Double}} are not related.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (THRIFT-4476) Typecasting problem on list items
[
https://issues.apache.org/jira/browse/THRIFT-4476?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16403845#comment-16403845
]
ASF GitHub Bot commented on THRIFT-4476:
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1496#discussion_r175276410
--- Diff: compiler/cpp/src/thrift/generate/t_generator.h ---
@@ -268,6 +271,30 @@ class t_generator {
return out.str();
}
+ const std::string emit_double_as_string(const double value) {
+ std::stringstream double_output_stream;
+ // sets the maximum precision:
http://en.cppreference.com/w/cpp/io/manip/setprecision
+ // sets the output format to fixed:
http://en.cppreference.com/w/cpp/io/manip/fixed (not in scientific notation)
+ double_output_stream <<
std::setprecision(std::numeric_limits::digits10 + 1);
+
+ #ifdef _MSC_VER
+ // strtod is broken in MSVC compilers older than 2015, so
std::fixed fails to format a double literal.
+ // more details:
https://blogs.msdn.microsoft.com/vcblog/2014/06/18/
+ //
c-runtime-crt-features-fixes-and-breaking-changes-in-visual-studio-14-ctp1/
+ // and
+ //
http://www.exploringbinary.com/visual-c-plus-plus-strtod-still-broken/
+ #if _MSC_VER >= MSC_2015_VER
+ double_output_stream << std::fixed;
+ #endif
+ #else
+ double_output_stream << std::fixed;
--- End diff --
Did you mean both cases to be the same? Both use std::fixed; should one be
using std::scientific?
> Typecasting problem on list items
> -
>
> Key: THRIFT-4476
> URL: https://issues.apache.org/jira/browse/THRIFT-4476
> Project: Thrift
> Issue Type: Bug
> Components: Java - Compiler
>Affects Versions: 0.11.0
>Reporter: Ozan Can Altiok
>Priority: Major
>
> I was trying to add the following into a thrift interface file.
> {{const list timeCoefficients = [24, 60, 60, 1000, 1000, 1000]}}
> With the definition given above the {{.thrift}} file compiled properly.
> However, I noticed that in Python, the items in {{timeCoefficients}} are
> considered to be integer although the list has been defined to include items
> of {{double}} type. Then I modified the definition as given below to make
> sure all the items are of type {{double}}.
> {{const list timeCoefficients = [24.0, 60.0, 60.0, 1000.0, 1000.0,
> 1000.0]}}
> After the change, I ran into the following error during compilation.
> {{[ERROR] .../TimeCoefficients.java:[402,48] error: no suitable method found
> for add(int)}}
> {{[ERROR] method Collection.add(Double) is not applicable}}
> {{[ERROR] (argument mismatch; int cannot be converted to Double)}}
> {{[ERROR] method List.add(Double) is not applicable}}
> {{[ERROR] (argument mismatch; int cannot be converted to Double)}}
> Next, I changed the line as follows and the compilation became successful.
> {{const list timeCoefficients = [24.1, 60.1, 60.1, 1000.1, 1000.1,
> 1000.1]}}
> When I reviewed the generated Java source files, I discovered that
> {{const list timeCoefficients = [24, 60, 60, 1000, 1000, 1000]}}
> compiles to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add((double)24);}}
> {{ timeCoefficients.add((double)60);}}
> {{ timeCoefficients.add((double)60);}}
> {{ timeCoefficients.add((double)1000);}}
> {{ timeCoefficients.add((double)1000);}}
> {{ timeCoefficients.add((double)1000);}}
> {{}}}
> whilst
> {{const list timeCoefficients = [24.0, 60.0, 60.0, 1000.0, 1000.0,
> 1000.0]}}
> compiles to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add(24);}}
> {{ timeCoefficients.add(60);}}
> {{ timeCoefficients.add(60);}}
> {{ timeCoefficients.add(1000);}}
> {{ timeCoefficients.add(1000);}}
> {{ timeCoefficients.add(1000);}}
> {{}}}
> which leads to an error.
> When I modified this line as follows
> {{const list timeCoefficients = [24.1, 60.1, 60.1, 1000.1, 1000.1,
> 1000.1]}}
> this line compiled to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add(24.1);}}
> {{ timeCoefficients.add(60.1);}}
> {{ timeCoefficients.add(60.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{}}}
> My guess is that, even if I put {{.0}} at the end of each numeric constant,
> on the Java side, Thrift compiler considers them to be {{double}} and does no
> typecasts. However, the {{.0}} s are getting lost somewher
[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...
Github user jeking3 commented on a diff in the pull request: https://github.com/apache/thrift/pull/1496#discussion_r175276394 --- Diff: build/appveyor/MSVC-appveyor-build.bat --- @@ -39,7 +39,7 @@ CD "%BUILDDIR%" || EXIT /B -DWITH_PYTHON=%WITH_PYTHON% ^ -DWITH_%THREADMODEL%THREADS=ON ^ -DWITH_SHARED_LIB=OFF ^ --DWITH_STATIC_LIB=ON|| EXIT /B +-DWITH_STATIC_LIB=ON ^|| EXIT /B --- End diff -- The carat addition looks wrong to me; it belongs on the end of a string, so in this case I think it does nothing at all? ---
[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1496#discussion_r175276410
--- Diff: compiler/cpp/src/thrift/generate/t_generator.h ---
@@ -268,6 +271,30 @@ class t_generator {
return out.str();
}
+ const std::string emit_double_as_string(const double value) {
+ std::stringstream double_output_stream;
+ // sets the maximum precision:
http://en.cppreference.com/w/cpp/io/manip/setprecision
+ // sets the output format to fixed:
http://en.cppreference.com/w/cpp/io/manip/fixed (not in scientific notation)
+ double_output_stream <<
std::setprecision(std::numeric_limits::digits10 + 1);
+
+ #ifdef _MSC_VER
+ // strtod is broken in MSVC compilers older than 2015, so
std::fixed fails to format a double literal.
+ // more details:
https://blogs.msdn.microsoft.com/vcblog/2014/06/18/
+ //
c-runtime-crt-features-fixes-and-breaking-changes-in-visual-studio-14-ctp1/
+ // and
+ //
http://www.exploringbinary.com/visual-c-plus-plus-strtod-still-broken/
+ #if _MSC_VER >= MSC_2015_VER
+ double_output_stream << std::fixed;
+ #endif
+ #else
+ double_output_stream << std::fixed;
--- End diff --
Did you mean both cases to be the same? Both use std::fixed; should one be
using std::scientific?
---
[jira] [Commented] (THRIFT-4419) Rust framed transport cannot handle writes above 4096 bytes
[
https://issues.apache.org/jira/browse/THRIFT-4419?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16403842#comment-16403842
]
James E. King, III commented on THRIFT-4419:
Nice, I like seeing cross tests re-enabled!
> Rust framed transport cannot handle writes above 4096 bytes
> ---
>
> Key: THRIFT-4419
> URL: https://issues.apache.org/jira/browse/THRIFT-4419
> Project: Thrift
> Issue Type: Bug
> Components: Rust - Library
>Reporter: Allen George
>Assignee: Allen George
>Priority: Critical
> Fix For: 0.12.0
>
>
> Related to THRIFT-4390
> Description copied form there:
> While working on improving test coverage and fixing busted cross tests I
> reworked the cpp test client to send binary in at size 0, 1, 2, 4, 6, 16,
> ..., 131072 and after 4096 the rust server gave up.
> {noformat}
> 12, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126,
> 127, 128])
> WARN:thrift::server::threaded: processor completed with error: TransportError
> { kind: Unknown, message: "failed to write whole buffer" }
> Server process is successfully killed.
> {noformat}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (THRIFT-4187) Dart -> Rust Framed cross tests fail
[ https://issues.apache.org/jira/browse/THRIFT-4187?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16403473#comment-16403473 ] ASF GitHub Bot commented on THRIFT-4187: Github user allengeorge commented on the issue: https://github.com/apache/thrift/pull/1269 @jeking3 Fixed up the failing tests. Confirming that precross works. > Dart -> Rust Framed cross tests fail > > > Key: THRIFT-4187 > URL: https://issues.apache.org/jira/browse/THRIFT-4187 > Project: Thrift > Issue Type: Bug > Components: Rust - Library >Reporter: Allen George >Assignee: Allen George >Priority: Minor > > For some reason the Dart (client) -> Rust (server) framed-transport cross > tests fail. Initial investigation shows that *somehow* the dart client think > the socket was closed, which means it doesn't read the message from the > underlying transport. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] thrift issue #1269: THRIFT-4187 Allow dart framed transport to read incomple...
Github user allengeorge commented on the issue: https://github.com/apache/thrift/pull/1269 @jeking3 Fixed up the failing tests. Confirming that precross works. ---
[jira] [Commented] (THRIFT-4476) Typecasting problem on list items
[
https://issues.apache.org/jira/browse/THRIFT-4476?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16403471#comment-16403471
]
ASF GitHub Bot commented on THRIFT-4476:
Github user ozymaxx commented on the issue:
https://github.com/apache/thrift/pull/1496
@jeking3 the build results have come out.
> Typecasting problem on list items
> -
>
> Key: THRIFT-4476
> URL: https://issues.apache.org/jira/browse/THRIFT-4476
> Project: Thrift
> Issue Type: Bug
> Components: Java - Compiler
>Affects Versions: 0.11.0
>Reporter: Ozan Can Altiok
>Priority: Major
>
> I was trying to add the following into a thrift interface file.
> {{const list timeCoefficients = [24, 60, 60, 1000, 1000, 1000]}}
> With the definition given above the {{.thrift}} file compiled properly.
> However, I noticed that in Python, the items in {{timeCoefficients}} are
> considered to be integer although the list has been defined to include items
> of {{double}} type. Then I modified the definition as given below to make
> sure all the items are of type {{double}}.
> {{const list timeCoefficients = [24.0, 60.0, 60.0, 1000.0, 1000.0,
> 1000.0]}}
> After the change, I ran into the following error during compilation.
> {{[ERROR] .../TimeCoefficients.java:[402,48] error: no suitable method found
> for add(int)}}
> {{[ERROR] method Collection.add(Double) is not applicable}}
> {{[ERROR] (argument mismatch; int cannot be converted to Double)}}
> {{[ERROR] method List.add(Double) is not applicable}}
> {{[ERROR] (argument mismatch; int cannot be converted to Double)}}
> Next, I changed the line as follows and the compilation became successful.
> {{const list timeCoefficients = [24.1, 60.1, 60.1, 1000.1, 1000.1,
> 1000.1]}}
> When I reviewed the generated Java source files, I discovered that
> {{const list timeCoefficients = [24, 60, 60, 1000, 1000, 1000]}}
> compiles to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add((double)24);}}
> {{ timeCoefficients.add((double)60);}}
> {{ timeCoefficients.add((double)60);}}
> {{ timeCoefficients.add((double)1000);}}
> {{ timeCoefficients.add((double)1000);}}
> {{ timeCoefficients.add((double)1000);}}
> {{}}}
> whilst
> {{const list timeCoefficients = [24.0, 60.0, 60.0, 1000.0, 1000.0,
> 1000.0]}}
> compiles to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add(24);}}
> {{ timeCoefficients.add(60);}}
> {{ timeCoefficients.add(60);}}
> {{ timeCoefficients.add(1000);}}
> {{ timeCoefficients.add(1000);}}
> {{ timeCoefficients.add(1000);}}
> {{}}}
> which leads to an error.
> When I modified this line as follows
> {{const list timeCoefficients = [24.1, 60.1, 60.1, 1000.1, 1000.1,
> 1000.1]}}
> this line compiled to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add(24.1);}}
> {{ timeCoefficients.add(60.1);}}
> {{ timeCoefficients.add(60.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{}}}
> My guess is that, even if I put {{.0}} at the end of each numeric constant,
> on the Java side, Thrift compiler considers them to be {{double}} and does no
> typecasts. However, the {{.0}} s are getting lost somewhere and the items
> become integers in the generated Java code. Thus, when it comes to populating
> the array, Java cannot succeed. {{Double}} s cannot be unboxed to integer and
> Java thinks {{int}} and {{Double}} are not related.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...
Github user ozymaxx commented on the issue: https://github.com/apache/thrift/pull/1496 @jeking3 the build results have come out. ---
[jira] [Commented] (THRIFT-4476) Typecasting problem on list items
[ https://issues.apache.org/jira/browse/THRIFT-4476?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16403451#comment-16403451 ] ASF GitHub Bot commented on THRIFT-4476: GitHub user ozymaxx reopened a pull request: https://github.com/apache/thrift/pull/1496 THRIFT-4476: Typecasting problem on list items (+ low precision double rendering, + mis-rendering non-fractional double literals in Python) - high precision double rendering in JS, Java, Python and C++ - testing double rendering in JS, Java, Python and C++ see the related issue: https://issues.apache.org/jira/browse/THRIFT-4476 You can merge this pull request into a Git repository by running: $ git pull https://github.com/ozymaxx/thrift THRIFT-4476 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/1496.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1496 commit f5a6337d4448a30c284a3033307865dfc992ac7d Author: Ozan Can Altiok Date: 2018-02-19T13:55:09Z - high precision double rendering in JS, Java, Python and C++ - testing double rendering in JS, Java, Python and C++ commit 6dbc35f74fe53a9bacf06867036c21f2761c823e Author: Ozan Can Altiok Date: 2018-02-22T09:47:27Z - a comment added (about setprecision) - BOOST_TEST -> BOOST_CHECK commit 55e3371ed9ea7eadc5826a7f17d3e432e9b99858 Author: Ozan Can Altiok Date: 2018-02-23T14:10:15Z double rendering in erlang, fixed (some additional tests added) commit 36cc6f9dfe5b508a531ad70958ca3209c3b65e84 Author: Ozan Can Altiok Date: 2018-02-23T14:44:31Z less than operator - correction in erlang test commit a22d72f024bbf463895022792a31d04054a79445 Author: Ozan Can Altiok Date: 2018-02-26T06:17:16Z style changes, some fixes in erlang test commit a615812729096a7fa106f8259eb2f41d61ac99ce Author: Ozan Can Altiok Date: 2018-02-26T06:51:24Z some fixes in erlang test commit 50b9a3fc69b82a9af2e78602a2e60b9a649874ee Author: Ozan Can Altiok Date: 2018-02-26T07:22:41Z some fixes in erlang test - no need for including lists library commit b4e9ce56aecf75beffbf77864f02891808072204 Author: Ozan Can Altiok Date: 2018-02-26T09:31:46Z more style changes - something overlooked commit 39bb546d0ccbec7177866a1a78d9b5f4de0d9ec1 Author: Ozan Can Altiok Date: 2018-02-26T10:24:45Z expected 2 lines before main - fixed (not seen locally) commit 98db81883af06d10e9277fb32988edfd3f044d1d Author: Ozan Can Altiok Date: 2018-02-26T11:24:05Z noticed that test cases should be added manually in Python commit 64aae330efac4ee23ed01694bc7dceb5a7a9164b Author: Ozan Can Altiok Date: 2018-02-27T06:48:48Z DoubleConstantTest thrift compilation directive is now given in generate.cmake commit decb436f40990e0a04be9730accd3becab951c39 Author: Ozan Can Altiok Date: 2018-02-28T06:13:34Z render_double_to_string -> emit_double_as_string commit e9b2e389b1184efa4d8ad1d6424ec52a108b80c9 Author: Ozan Can Altiok Date: 2018-03-02T12:45:54Z added assertion error messages to see the problems inside Java tests in more detail commit 248e8a4c208e518da30f761a2d836ff790cd9b0d Author: Ozan Can Altiok Date: 2018-03-05T06:59:59Z RDP access to AppVeyor build workers commit dd9b65113f143f13e6524917add6c216b16f86e5 Author: Ozan Can Altiok Date: 2018-03-05T07:01:07Z Merge branch 'master' of https://github.com/apache/thrift into THRIFT-4476 commit 65a6588d049dcbbef7d3df5d7c3ba9a7b141917a Author: Ozan Can Altiok Date: 2018-03-05T07:03:20Z duplicate key problem in appveyor settings commit 6305384d9203bd532983d18a992515db802b29a5 Author: Ozan Can Altiok Date: 2018-03-05T09:31:22Z ignore the test cases with (-+)1.7e+308 in Java tests commit 74667acf55c435c661c5fc19de729eb5f82758dc Author: Ozan Can Altiok Date: 2018-03-05T10:00:31Z ignore the test cases with (-+)1.7e+308 in Java tests - 2 commit 85b872bac39078446aa27336ca57001022524fc0 Author: Ozan Can Altiok Date: 2018-03-05T12:18:14Z pyflakes8 style changes commit af11ef8c7475c5f57f2d6619bedea23856a0fd17 Author: Ozan Can Altiok Date: 2018-03-06T08:21:51Z disabling the test named 'TestRenderedDoubleConstants' on machines with the MSVC2010 configuration commit 8ed1997141dce6c4b4505b09d2d6843344dfa04b Author: Ozan Can Altiok Date: 2018-03-06T08:58:25Z msvc.profile parameter existence check commit beb12744f990ed0890648b07d81f19c121303c59 Author: Ozan Can Altiok Date: 2018-03-06T12:05:04Z passing "msvc profile" on build stage commit 1a0756aa8d48b693ae39dd860bdbeb7de5252cae Author: Ozan Can Altiok Date: 2018-03-06T12:50:10Z "MSVC_PROFILE" as a cache variable commit b40fc4e06083f2787db3f805756abe7e6c2c704d Author: Ozan Can Altiok Date: 2018-03-0
[GitHub] thrift issue #1496: THRIFT-4476: Typecasting problem on list items (+ low pr...
Github user ozymaxx commented on the issue: https://github.com/apache/thrift/pull/1496 I have reverted the changes on passing the compiler flag to the testers, and modified the double emitter. ---
[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...
GitHub user ozymaxx reopened a pull request: https://github.com/apache/thrift/pull/1496 THRIFT-4476: Typecasting problem on list items (+ low precision double rendering, + mis-rendering non-fractional double literals in Python) - high precision double rendering in JS, Java, Python and C++ - testing double rendering in JS, Java, Python and C++ see the related issue: https://issues.apache.org/jira/browse/THRIFT-4476 You can merge this pull request into a Git repository by running: $ git pull https://github.com/ozymaxx/thrift THRIFT-4476 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/1496.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1496 commit f5a6337d4448a30c284a3033307865dfc992ac7d Author: Ozan Can Altiok Date: 2018-02-19T13:55:09Z - high precision double rendering in JS, Java, Python and C++ - testing double rendering in JS, Java, Python and C++ commit 6dbc35f74fe53a9bacf06867036c21f2761c823e Author: Ozan Can Altiok Date: 2018-02-22T09:47:27Z - a comment added (about setprecision) - BOOST_TEST -> BOOST_CHECK commit 55e3371ed9ea7eadc5826a7f17d3e432e9b99858 Author: Ozan Can Altiok Date: 2018-02-23T14:10:15Z double rendering in erlang, fixed (some additional tests added) commit 36cc6f9dfe5b508a531ad70958ca3209c3b65e84 Author: Ozan Can Altiok Date: 2018-02-23T14:44:31Z less than operator - correction in erlang test commit a22d72f024bbf463895022792a31d04054a79445 Author: Ozan Can Altiok Date: 2018-02-26T06:17:16Z style changes, some fixes in erlang test commit a615812729096a7fa106f8259eb2f41d61ac99ce Author: Ozan Can Altiok Date: 2018-02-26T06:51:24Z some fixes in erlang test commit 50b9a3fc69b82a9af2e78602a2e60b9a649874ee Author: Ozan Can Altiok Date: 2018-02-26T07:22:41Z some fixes in erlang test - no need for including lists library commit b4e9ce56aecf75beffbf77864f02891808072204 Author: Ozan Can Altiok Date: 2018-02-26T09:31:46Z more style changes - something overlooked commit 39bb546d0ccbec7177866a1a78d9b5f4de0d9ec1 Author: Ozan Can Altiok Date: 2018-02-26T10:24:45Z expected 2 lines before main - fixed (not seen locally) commit 98db81883af06d10e9277fb32988edfd3f044d1d Author: Ozan Can Altiok Date: 2018-02-26T11:24:05Z noticed that test cases should be added manually in Python commit 64aae330efac4ee23ed01694bc7dceb5a7a9164b Author: Ozan Can Altiok Date: 2018-02-27T06:48:48Z DoubleConstantTest thrift compilation directive is now given in generate.cmake commit decb436f40990e0a04be9730accd3becab951c39 Author: Ozan Can Altiok Date: 2018-02-28T06:13:34Z render_double_to_string -> emit_double_as_string commit e9b2e389b1184efa4d8ad1d6424ec52a108b80c9 Author: Ozan Can Altiok Date: 2018-03-02T12:45:54Z added assertion error messages to see the problems inside Java tests in more detail commit 248e8a4c208e518da30f761a2d836ff790cd9b0d Author: Ozan Can Altiok Date: 2018-03-05T06:59:59Z RDP access to AppVeyor build workers commit dd9b65113f143f13e6524917add6c216b16f86e5 Author: Ozan Can Altiok Date: 2018-03-05T07:01:07Z Merge branch 'master' of https://github.com/apache/thrift into THRIFT-4476 commit 65a6588d049dcbbef7d3df5d7c3ba9a7b141917a Author: Ozan Can Altiok Date: 2018-03-05T07:03:20Z duplicate key problem in appveyor settings commit 6305384d9203bd532983d18a992515db802b29a5 Author: Ozan Can Altiok Date: 2018-03-05T09:31:22Z ignore the test cases with (-+)1.7e+308 in Java tests commit 74667acf55c435c661c5fc19de729eb5f82758dc Author: Ozan Can Altiok Date: 2018-03-05T10:00:31Z ignore the test cases with (-+)1.7e+308 in Java tests - 2 commit 85b872bac39078446aa27336ca57001022524fc0 Author: Ozan Can Altiok Date: 2018-03-05T12:18:14Z pyflakes8 style changes commit af11ef8c7475c5f57f2d6619bedea23856a0fd17 Author: Ozan Can Altiok Date: 2018-03-06T08:21:51Z disabling the test named 'TestRenderedDoubleConstants' on machines with the MSVC2010 configuration commit 8ed1997141dce6c4b4505b09d2d6843344dfa04b Author: Ozan Can Altiok Date: 2018-03-06T08:58:25Z msvc.profile parameter existence check commit beb12744f990ed0890648b07d81f19c121303c59 Author: Ozan Can Altiok Date: 2018-03-06T12:05:04Z passing "msvc profile" on build stage commit 1a0756aa8d48b693ae39dd860bdbeb7de5252cae Author: Ozan Can Altiok Date: 2018-03-06T12:50:10Z "MSVC_PROFILE" as a cache variable commit b40fc4e06083f2787db3f805756abe7e6c2c704d Author: Ozan Can Altiok Date: 2018-03-06T13:01:48Z wrong usage of cache variables commit ef43030fd69f1dee060c3cf447acfcceefb4be71 Author: Ozan Can Altiok Date: 2018-03-06T13:19:42Z let's see whether the MSVC_PROFILE parameter is passed correctly commit ec32218d28d6054f6fa5148c0d5d24
[jira] [Commented] (THRIFT-4476) Typecasting problem on list items
[
https://issues.apache.org/jira/browse/THRIFT-4476?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16403450#comment-16403450
]
ASF GitHub Bot commented on THRIFT-4476:
Github user ozymaxx commented on the issue:
https://github.com/apache/thrift/pull/1496
I have reverted the changes on passing the compiler flag to the testers,
and modified the double emitter.
> Typecasting problem on list items
> -
>
> Key: THRIFT-4476
> URL: https://issues.apache.org/jira/browse/THRIFT-4476
> Project: Thrift
> Issue Type: Bug
> Components: Java - Compiler
>Affects Versions: 0.11.0
>Reporter: Ozan Can Altiok
>Priority: Major
>
> I was trying to add the following into a thrift interface file.
> {{const list timeCoefficients = [24, 60, 60, 1000, 1000, 1000]}}
> With the definition given above the {{.thrift}} file compiled properly.
> However, I noticed that in Python, the items in {{timeCoefficients}} are
> considered to be integer although the list has been defined to include items
> of {{double}} type. Then I modified the definition as given below to make
> sure all the items are of type {{double}}.
> {{const list timeCoefficients = [24.0, 60.0, 60.0, 1000.0, 1000.0,
> 1000.0]}}
> After the change, I ran into the following error during compilation.
> {{[ERROR] .../TimeCoefficients.java:[402,48] error: no suitable method found
> for add(int)}}
> {{[ERROR] method Collection.add(Double) is not applicable}}
> {{[ERROR] (argument mismatch; int cannot be converted to Double)}}
> {{[ERROR] method List.add(Double) is not applicable}}
> {{[ERROR] (argument mismatch; int cannot be converted to Double)}}
> Next, I changed the line as follows and the compilation became successful.
> {{const list timeCoefficients = [24.1, 60.1, 60.1, 1000.1, 1000.1,
> 1000.1]}}
> When I reviewed the generated Java source files, I discovered that
> {{const list timeCoefficients = [24, 60, 60, 1000, 1000, 1000]}}
> compiles to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add((double)24);}}
> {{ timeCoefficients.add((double)60);}}
> {{ timeCoefficients.add((double)60);}}
> {{ timeCoefficients.add((double)1000);}}
> {{ timeCoefficients.add((double)1000);}}
> {{ timeCoefficients.add((double)1000);}}
> {{}}}
> whilst
> {{const list timeCoefficients = [24.0, 60.0, 60.0, 1000.0, 1000.0,
> 1000.0]}}
> compiles to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add(24);}}
> {{ timeCoefficients.add(60);}}
> {{ timeCoefficients.add(60);}}
> {{ timeCoefficients.add(1000);}}
> {{ timeCoefficients.add(1000);}}
> {{ timeCoefficients.add(1000);}}
> {{}}}
> which leads to an error.
> When I modified this line as follows
> {{const list timeCoefficients = [24.1, 60.1, 60.1, 1000.1, 1000.1,
> 1000.1]}}
> this line compiled to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add(24.1);}}
> {{ timeCoefficients.add(60.1);}}
> {{ timeCoefficients.add(60.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{}}}
> My guess is that, even if I put {{.0}} at the end of each numeric constant,
> on the Java side, Thrift compiler considers them to be {{double}} and does no
> typecasts. However, the {{.0}} s are getting lost somewhere and the items
> become integers in the generated Java code. Thus, when it comes to populating
> the array, Java cannot succeed. {{Double}} s cannot be unboxed to integer and
> Java thinks {{int}} and {{Double}} are not related.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (THRIFT-4476) Typecasting problem on list items
[
https://issues.apache.org/jira/browse/THRIFT-4476?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16403388#comment-16403388
]
ASF GitHub Bot commented on THRIFT-4476:
Github user ozymaxx closed the pull request at:
https://github.com/apache/thrift/pull/1496
> Typecasting problem on list items
> -
>
> Key: THRIFT-4476
> URL: https://issues.apache.org/jira/browse/THRIFT-4476
> Project: Thrift
> Issue Type: Bug
> Components: Java - Compiler
>Affects Versions: 0.11.0
>Reporter: Ozan Can Altiok
>Priority: Major
>
> I was trying to add the following into a thrift interface file.
> {{const list timeCoefficients = [24, 60, 60, 1000, 1000, 1000]}}
> With the definition given above the {{.thrift}} file compiled properly.
> However, I noticed that in Python, the items in {{timeCoefficients}} are
> considered to be integer although the list has been defined to include items
> of {{double}} type. Then I modified the definition as given below to make
> sure all the items are of type {{double}}.
> {{const list timeCoefficients = [24.0, 60.0, 60.0, 1000.0, 1000.0,
> 1000.0]}}
> After the change, I ran into the following error during compilation.
> {{[ERROR] .../TimeCoefficients.java:[402,48] error: no suitable method found
> for add(int)}}
> {{[ERROR] method Collection.add(Double) is not applicable}}
> {{[ERROR] (argument mismatch; int cannot be converted to Double)}}
> {{[ERROR] method List.add(Double) is not applicable}}
> {{[ERROR] (argument mismatch; int cannot be converted to Double)}}
> Next, I changed the line as follows and the compilation became successful.
> {{const list timeCoefficients = [24.1, 60.1, 60.1, 1000.1, 1000.1,
> 1000.1]}}
> When I reviewed the generated Java source files, I discovered that
> {{const list timeCoefficients = [24, 60, 60, 1000, 1000, 1000]}}
> compiles to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add((double)24);}}
> {{ timeCoefficients.add((double)60);}}
> {{ timeCoefficients.add((double)60);}}
> {{ timeCoefficients.add((double)1000);}}
> {{ timeCoefficients.add((double)1000);}}
> {{ timeCoefficients.add((double)1000);}}
> {{}}}
> whilst
> {{const list timeCoefficients = [24.0, 60.0, 60.0, 1000.0, 1000.0,
> 1000.0]}}
> compiles to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add(24);}}
> {{ timeCoefficients.add(60);}}
> {{ timeCoefficients.add(60);}}
> {{ timeCoefficients.add(1000);}}
> {{ timeCoefficients.add(1000);}}
> {{ timeCoefficients.add(1000);}}
> {{}}}
> which leads to an error.
> When I modified this line as follows
> {{const list timeCoefficients = [24.1, 60.1, 60.1, 1000.1, 1000.1,
> 1000.1]}}
> this line compiled to
> {{public static final java.util.List timeCoefficients = new
> java.util.ArrayList();}}
> {{static {}}
> {{ timeCoefficients.add(24.1);}}
> {{ timeCoefficients.add(60.1);}}
> {{ timeCoefficients.add(60.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{ timeCoefficients.add(1000.1);}}
> {{}}}
> My guess is that, even if I put {{.0}} at the end of each numeric constant,
> on the Java side, Thrift compiler considers them to be {{double}} and does no
> typecasts. However, the {{.0}} s are getting lost somewhere and the items
> become integers in the generated Java code. Thus, when it comes to populating
> the array, Java cannot succeed. {{Double}} s cannot be unboxed to integer and
> Java thinks {{int}} and {{Double}} are not related.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[GitHub] thrift pull request #1496: THRIFT-4476: Typecasting problem on list items (+...
Github user ozymaxx closed the pull request at: https://github.com/apache/thrift/pull/1496 ---
[GitHub] thrift issue #1440: Enhancement binary_protocol with frametransport
Github user dcelasun commented on the issue: https://github.com/apache/thrift/pull/1440 From a cursory look, while the performance improvements are decent and only small code changes are needed, this doesn't really fit into our current architecture. Since the new types don't depend on any unexported types we define, I think this should be a separate library. ---
[GitHub] thrift pull request #1440: Enhancement binary_protocol with frametransport
Github user dcelasun commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1440#discussion_r175252307
--- Diff: lib/go/thrift/fast_buffer.go ---
@@ -0,0 +1,257 @@
+package thrift
+
+import (
+ "fmt"
+ "unsafe"
+)
+
+type FastFrameBuffer struct {
+ w *TFramedTransport //
+ b []byte
+ wIdx int // write-offset, reset when Flush
+ frameSize int
+ rIdx int // read-offset, reset when read-new-frame
+ buf [4]byte // only for write&read data-len
+}
+
+func NewFastBuffer(w *TFramedTransport, size int) *FastFrameBuffer {
+ return &FastFrameBuffer{
+ w:w,
+ b:make([]byte, size),
+ rIdx: 0,
+ wIdx: 0,
+ }
+}
+
+func (p *FastFrameBuffer) WritByte(b int8) {
+ if p.wIdx+1 > p.Cap() {
+ p.grow(2*p.Cap() + 1)
+ }
+ p.b[p.wIdx] = byte(b)
+ p.wIdx += 1
+}
+
+func (p *FastFrameBuffer) WriteBytes(b []byte) {
+ if p.wIdx+len(b) > p.Cap() {
+ p.grow(2*p.Cap() + len(b))
+ }
+ copy(p.b[p.wIdx:], b)
+ p.wIdx += len(b)
+}
+
+func (p *FastFrameBuffer) WriteI16(i uint16) {
+ if p.wIdx+2 > p.Cap() {
+ p.grow(2*p.Cap() + 2)
+ }
+ copy(p.b[p.wIdx:], []byte{byte(i >> 8), byte(i)})
+ p.wIdx += 2
+}
+
+func (p *FastFrameBuffer) WriteI32(i uint32) {
+ if p.wIdx+4 > p.Cap() {
+ p.grow(2*p.Cap() + 4)
+ }
+ copy(p.b[p.wIdx:], []byte{byte(i >> 24), byte(i >> 16), byte(i >> 8),
byte(i)})
+ p.wIdx += 4
+}
+
+func (p *FastFrameBuffer) WriteI64(i uint64) {
+ if p.wIdx+8 > p.Cap() {
+ p.grow(2*p.Cap() + 8)
+ }
+ copy(p.b[p.wIdx:], []byte{byte(i >> 56), byte(i >> 48), byte(i >> 40),
byte(i >> 32), byte(i >> 24), byte(i >> 16), byte(i >> 8), byte(i)})
+ p.wIdx += 8
+}
+
+func (p *FastFrameBuffer) WriteString(s string) {
+ if p.wIdx+len(s) > p.Cap() {
+ p.grow(2*p.Cap() + len(s))
+ }
+ copy(p.b[p.wIdx:], str2bytes(s))
+ p.wIdx += len(s)
+}
+
+func (p *FastFrameBuffer) Len() int {
+ return len(p.b)
+}
+
+func (p *FastFrameBuffer) Cap() int {
+ return cap(p.b)
+}
+
+func (p *FastFrameBuffer) Flush() error {
+ p.buf[0] = byte(p.wIdx >> 24)
+ p.buf[1] = byte(p.wIdx >> 16)
+ p.buf[2] = byte(p.wIdx >> 8)
+ p.buf[3] = byte(p.wIdx)
+ _, err := p.w.transport.Write(p.buf[:4])
+
+ if err != nil {
+ return fmt.Errorf("Flush Write-Len failed, err: %v\n", err)
+ }
+ _, err = p.w.transport.Write(p.b[:p.wIdx])
+ if err != nil {
+ return fmt.Errorf("Flush Write-Dat failed, err: %v\n", err)
+ }
+ p.ResetWriter()
+ p.w.transport.Flush()
+ return nil
+}
+
+func (p *FastFrameBuffer) ResetWriter() {
+ p.wIdx = 0
+}
+
+func (p *FastFrameBuffer) ResetReader() {
+ p.rIdx = 0
+}
+
+func (p *FastFrameBuffer) grow(n int) {
+ b := make([]byte, n)
+ copy(b, p.b[0:])
+ p.b = b
+}
+
+func (p *FastFrameBuffer) ReadByte() (c byte, err error) {
+ if p.frameSize == 0 {
+ p.frameSize, err = p.readFrameHeader()
+ if err != nil {
+ return
+ }
+ _, err = p.readAll(p.frameSize)
+ if err != nil {
+ return
+ }
+ }
+ if p.frameSize < 1 {
+ return 0, fmt.Errorf("Not enought frame size %d to read %d
bytes", p.frameSize, 1)
--- End diff --
s/enought/enough everywhere in the file.
---
[GitHub] thrift pull request #1440: Enhancement binary_protocol with frametransport
Github user dcelasun commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1440#discussion_r175252297
--- Diff: lib/go/thrift/fast_buffer.go ---
@@ -0,0 +1,257 @@
+package thrift
+
+import (
+ "fmt"
+ "unsafe"
+)
+
+type FastFrameBuffer struct {
+ w *TFramedTransport //
+ b []byte
+ wIdx int // write-offset, reset when Flush
+ frameSize int
+ rIdx int // read-offset, reset when read-new-frame
+ buf [4]byte // only for write&read data-len
+}
+
+func NewFastBuffer(w *TFramedTransport, size int) *FastFrameBuffer {
+ return &FastFrameBuffer{
+ w:w,
+ b:make([]byte, size),
+ rIdx: 0,
+ wIdx: 0,
+ }
+}
+
+func (p *FastFrameBuffer) WritByte(b int8) {
+ if p.wIdx+1 > p.Cap() {
+ p.grow(2*p.Cap() + 1)
+ }
+ p.b[p.wIdx] = byte(b)
+ p.wIdx += 1
+}
+
+func (p *FastFrameBuffer) WriteBytes(b []byte) {
+ if p.wIdx+len(b) > p.Cap() {
+ p.grow(2*p.Cap() + len(b))
+ }
+ copy(p.b[p.wIdx:], b)
+ p.wIdx += len(b)
+}
+
+func (p *FastFrameBuffer) WriteI16(i uint16) {
+ if p.wIdx+2 > p.Cap() {
+ p.grow(2*p.Cap() + 2)
+ }
+ copy(p.b[p.wIdx:], []byte{byte(i >> 8), byte(i)})
+ p.wIdx += 2
+}
+
+func (p *FastFrameBuffer) WriteI32(i uint32) {
+ if p.wIdx+4 > p.Cap() {
+ p.grow(2*p.Cap() + 4)
+ }
+ copy(p.b[p.wIdx:], []byte{byte(i >> 24), byte(i >> 16), byte(i >> 8),
byte(i)})
+ p.wIdx += 4
+}
+
+func (p *FastFrameBuffer) WriteI64(i uint64) {
+ if p.wIdx+8 > p.Cap() {
+ p.grow(2*p.Cap() + 8)
+ }
+ copy(p.b[p.wIdx:], []byte{byte(i >> 56), byte(i >> 48), byte(i >> 40),
byte(i >> 32), byte(i >> 24), byte(i >> 16), byte(i >> 8), byte(i)})
+ p.wIdx += 8
+}
+
+func (p *FastFrameBuffer) WriteString(s string) {
+ if p.wIdx+len(s) > p.Cap() {
+ p.grow(2*p.Cap() + len(s))
+ }
+ copy(p.b[p.wIdx:], str2bytes(s))
+ p.wIdx += len(s)
+}
+
+func (p *FastFrameBuffer) Len() int {
+ return len(p.b)
+}
+
+func (p *FastFrameBuffer) Cap() int {
+ return cap(p.b)
+}
+
+func (p *FastFrameBuffer) Flush() error {
+ p.buf[0] = byte(p.wIdx >> 24)
+ p.buf[1] = byte(p.wIdx >> 16)
+ p.buf[2] = byte(p.wIdx >> 8)
+ p.buf[3] = byte(p.wIdx)
+ _, err := p.w.transport.Write(p.buf[:4])
+
+ if err != nil {
+ return fmt.Errorf("Flush Write-Len failed, err: %v\n", err)
+ }
+ _, err = p.w.transport.Write(p.b[:p.wIdx])
+ if err != nil {
+ return fmt.Errorf("Flush Write-Dat failed, err: %v\n", err)
+ }
+ p.ResetWriter()
+ p.w.transport.Flush()
+ return nil
+}
+
+func (p *FastFrameBuffer) ResetWriter() {
+ p.wIdx = 0
+}
+
+func (p *FastFrameBuffer) ResetReader() {
+ p.rIdx = 0
+}
+
+func (p *FastFrameBuffer) grow(n int) {
+ b := make([]byte, n)
+ copy(b, p.b[0:])
+ p.b = b
+}
+
+func (p *FastFrameBuffer) ReadByte() (c byte, err error) {
+ if p.frameSize == 0 {
+ p.frameSize, err = p.readFrameHeader()
+ if err != nil {
+ return
+ }
+ _, err = p.readAll(p.frameSize)
+ if err != nil {
+ return
+ }
+ }
+ if p.frameSize < 1 {
+ return 0, fmt.Errorf("Not enought frame size %d to read %d
bytes", p.frameSize, 1)
+ }
+ c = p.b[p.rIdx]
+ if err == nil {
+ p.frameSize--
+ p.rIdx += 1
+ }
+ return
+}
+
+// maybe read-bytes means ReadN
+func (p *FastFrameBuffer) ReadN(num int) (b []byte, err error) {
+ if p.frameSize == 0 {
+ p.frameSize, err = p.readFrameHeader()
+ if err != nil {
+ return
+ }
+ _, err = p.readAll(p.frameSize)
+ if err != nil {
+ return
+ }
+ }
+ if p.frameSize < num {
+ return nil, fmt.Errorf("Not enought frame size %d to read %d
bytes", p.frameSize, num)
+ }
+ b = p.b[p.rIdx : p.rIdx+num]
+ p.frameSize = p.frameSize - num
+ if p.frameSize < 0 {
+ return nil, fmt.Errorf("Negative frame size")
+ }
+ p.rIdx += num
+ return b, nil
+}
+
+func (p *FastFrameBuffer) ReadI64() (num int64, err error) {
+ if p.frameSize == 0 {
+ p.frameSize, err = p.readFrameHeader()
+ if err != nil {
+ return
+
[GitHub] thrift pull request #1440: Enhancement binary_protocol with frametransport
Github user dcelasun commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1440#discussion_r175252293
--- Diff: lib/go/thrift/fast_buffer.go ---
@@ -0,0 +1,257 @@
+package thrift
+
+import (
+ "fmt"
+ "unsafe"
+)
+
+type FastFrameBuffer struct {
+ w *TFramedTransport //
+ b []byte
+ wIdx int // write-offset, reset when Flush
+ frameSize int
+ rIdx int // read-offset, reset when read-new-frame
+ buf [4]byte // only for write&read data-len
+}
+
+func NewFastBuffer(w *TFramedTransport, size int) *FastFrameBuffer {
+ return &FastFrameBuffer{
+ w:w,
+ b:make([]byte, size),
+ rIdx: 0,
+ wIdx: 0,
+ }
+}
+
+func (p *FastFrameBuffer) WritByte(b int8) {
+ if p.wIdx+1 > p.Cap() {
+ p.grow(2*p.Cap() + 1)
+ }
+ p.b[p.wIdx] = byte(b)
+ p.wIdx += 1
+}
+
+func (p *FastFrameBuffer) WriteBytes(b []byte) {
+ if p.wIdx+len(b) > p.Cap() {
+ p.grow(2*p.Cap() + len(b))
+ }
+ copy(p.b[p.wIdx:], b)
+ p.wIdx += len(b)
+}
+
+func (p *FastFrameBuffer) WriteI16(i uint16) {
+ if p.wIdx+2 > p.Cap() {
+ p.grow(2*p.Cap() + 2)
+ }
+ copy(p.b[p.wIdx:], []byte{byte(i >> 8), byte(i)})
+ p.wIdx += 2
+}
+
+func (p *FastFrameBuffer) WriteI32(i uint32) {
+ if p.wIdx+4 > p.Cap() {
+ p.grow(2*p.Cap() + 4)
+ }
+ copy(p.b[p.wIdx:], []byte{byte(i >> 24), byte(i >> 16), byte(i >> 8),
byte(i)})
+ p.wIdx += 4
+}
+
+func (p *FastFrameBuffer) WriteI64(i uint64) {
+ if p.wIdx+8 > p.Cap() {
+ p.grow(2*p.Cap() + 8)
+ }
+ copy(p.b[p.wIdx:], []byte{byte(i >> 56), byte(i >> 48), byte(i >> 40),
byte(i >> 32), byte(i >> 24), byte(i >> 16), byte(i >> 8), byte(i)})
+ p.wIdx += 8
+}
+
+func (p *FastFrameBuffer) WriteString(s string) {
+ if p.wIdx+len(s) > p.Cap() {
+ p.grow(2*p.Cap() + len(s))
+ }
+ copy(p.b[p.wIdx:], str2bytes(s))
+ p.wIdx += len(s)
+}
+
+func (p *FastFrameBuffer) Len() int {
+ return len(p.b)
+}
+
+func (p *FastFrameBuffer) Cap() int {
+ return cap(p.b)
+}
+
+func (p *FastFrameBuffer) Flush() error {
+ p.buf[0] = byte(p.wIdx >> 24)
+ p.buf[1] = byte(p.wIdx >> 16)
+ p.buf[2] = byte(p.wIdx >> 8)
+ p.buf[3] = byte(p.wIdx)
+ _, err := p.w.transport.Write(p.buf[:4])
+
+ if err != nil {
+ return fmt.Errorf("Flush Write-Len failed, err: %v\n", err)
+ }
+ _, err = p.w.transport.Write(p.b[:p.wIdx])
+ if err != nil {
+ return fmt.Errorf("Flush Write-Dat failed, err: %v\n", err)
+ }
+ p.ResetWriter()
+ p.w.transport.Flush()
+ return nil
+}
+
+func (p *FastFrameBuffer) ResetWriter() {
+ p.wIdx = 0
+}
+
+func (p *FastFrameBuffer) ResetReader() {
+ p.rIdx = 0
+}
+
+func (p *FastFrameBuffer) grow(n int) {
+ b := make([]byte, n)
+ copy(b, p.b[0:])
+ p.b = b
+}
+
+func (p *FastFrameBuffer) ReadByte() (c byte, err error) {
+ if p.frameSize == 0 {
+ p.frameSize, err = p.readFrameHeader()
+ if err != nil {
+ return
+ }
+ _, err = p.readAll(p.frameSize)
+ if err != nil {
+ return
+ }
+ }
+ if p.frameSize < 1 {
+ return 0, fmt.Errorf("Not enought frame size %d to read %d
bytes", p.frameSize, 1)
+ }
+ c = p.b[p.rIdx]
+ if err == nil {
+ p.frameSize--
+ p.rIdx += 1
+ }
+ return
+}
+
+// maybe read-bytes means ReadN
+func (p *FastFrameBuffer) ReadN(num int) (b []byte, err error) {
+ if p.frameSize == 0 {
+ p.frameSize, err = p.readFrameHeader()
+ if err != nil {
+ return
+ }
+ _, err = p.readAll(p.frameSize)
+ if err != nil {
+ return
+ }
+ }
+ if p.frameSize < num {
+ return nil, fmt.Errorf("Not enought frame size %d to read %d
bytes", p.frameSize, num)
+ }
+ b = p.b[p.rIdx : p.rIdx+num]
+ p.frameSize = p.frameSize - num
+ if p.frameSize < 0 {
+ return nil, fmt.Errorf("Negative frame size")
+ }
+ p.rIdx += num
+ return b, nil
+}
+
+func (p *FastFrameBuffer) ReadI64() (num int64, err error) {
+ if p.frameSize == 0 {
+ p.frameSize, err = p.readFrameHeader()
+ if err != nil {
+ return
+
[jira] [Resolved] (THRIFT-4419) Rust framed transport cannot handle writes above 4096 bytes
[
https://issues.apache.org/jira/browse/THRIFT-4419?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Can Celasun resolved THRIFT-4419.
-
Resolution: Fixed
Fix Version/s: 0.12.0
> Rust framed transport cannot handle writes above 4096 bytes
> ---
>
> Key: THRIFT-4419
> URL: https://issues.apache.org/jira/browse/THRIFT-4419
> Project: Thrift
> Issue Type: Bug
> Components: Rust - Library
>Reporter: Allen George
>Assignee: Allen George
>Priority: Critical
> Fix For: 0.12.0
>
>
> Related to THRIFT-4390
> Description copied form there:
> While working on improving test coverage and fixing busted cross tests I
> reworked the cpp test client to send binary in at size 0, 1, 2, 4, 6, 16,
> ..., 131072 and after 4096 the rust server gave up.
> {noformat}
> 12, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126,
> 127, 128])
> WARN:thrift::server::threaded: processor completed with error: TransportError
> { kind: Unknown, message: "failed to write whole buffer" }
> Server process is successfully killed.
> {noformat}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (THRIFT-4419) Rust framed transport cannot handle writes above 4096 bytes
[
https://issues.apache.org/jira/browse/THRIFT-4419?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16403314#comment-16403314
]
ASF GitHub Bot commented on THRIFT-4419:
Github user asfgit closed the pull request at:
https://github.com/apache/thrift/pull/1508
> Rust framed transport cannot handle writes above 4096 bytes
> ---
>
> Key: THRIFT-4419
> URL: https://issues.apache.org/jira/browse/THRIFT-4419
> Project: Thrift
> Issue Type: Bug
> Components: Rust - Library
>Reporter: Allen George
>Assignee: Allen George
>Priority: Critical
>
> Related to THRIFT-4390
> Description copied form there:
> While working on improving test coverage and fixing busted cross tests I
> reworked the cpp test client to send binary in at size 0, 1, 2, 4, 6, 16,
> ..., 131072 and after 4096 the rust server gave up.
> {noformat}
> 12, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126,
> 127, 128])
> WARN:thrift::server::threaded: processor completed with error: TransportError
> { kind: Unknown, message: "failed to write whole buffer" }
> Server process is successfully killed.
> {noformat}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[GitHub] thrift pull request #1508: THRIFT-4419: Fix bug where framed messages > 4K c...
Github user asfgit closed the pull request at: https://github.com/apache/thrift/pull/1508 ---
