You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Bryan Duxbury (JIRA)" <ji...@apache.org> on 2009/02/10 06:40:59 UTC
[jira] Created: (THRIFT-333) Compact Protocol for C++
Compact Protocol for C++
------------------------
Key: THRIFT-333
URL: https://issues.apache.org/jira/browse/THRIFT-333
Project: Thrift
Issue Type: Sub-task
Components: Library (C++)
Reporter: Bryan Duxbury
Priority: Trivial
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-333) Compact Protocol for C++
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12688965#action_12688965 ]
David Reiss commented on THRIFT-333:
------------------------------------
Yeah, it should be the same as bitwise_cast. You should probably copy the boost_static_assert from TBinaryProtocol::{read,write}Double to ensure that the machine format is correct.
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Fix For: 0.1
>
> Attachments: q.cpp, THRIFT_333_CPP_Compact_Protocol.patch, THRIFT_333_CPP_Compact_Protocol.v2.patch, THRIFT_333_CPP_Compact_Protocol.v3.patch, THRIFT_333_CPP_Compact_Protocol_Tests.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-333) Compact Protocol for C++
Posted by "Chad Walters (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12688153#action_12688153 ]
Chad Walters commented on THRIFT-333:
-------------------------------------
Didn't look too closely at the code but I did notice that the Apache headers are not present in the new files (per http://www.apache.org/legal/src-headers.html).
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Fix For: 0.1
>
> Attachments: THRIFT_333_CPP_Compact_Protocol.patch, THRIFT_333_CPP_Compact_Protocol.v2.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-333) Compact Protocol for C++
Posted by "Jérémie BORDIER (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12689283#action_12689283 ]
Jérémie BORDIER commented on THRIFT-333:
----------------------------------------
Thanks a lot for your work David, that's perfect. LFGTM, +1 !
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Fix For: 0.1
>
> Attachments: q.cpp, t333-compact-v4.diff, THRIFT_333_CPP_Compact_Protocol.patch, THRIFT_333_CPP_Compact_Protocol.v2.patch, THRIFT_333_CPP_Compact_Protocol.v3.patch, THRIFT_333_CPP_Compact_Protocol_Tests.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (THRIFT-333) Compact Protocol for C++
Posted by "Jérémie BORDIER (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jérémie BORDIER reassigned THRIFT-333:
--------------------------------------
Assignee: Jérémie BORDIER
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-333) Compact Protocol for C++
Posted by "Jérémie BORDIER (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jérémie BORDIER updated THRIFT-333:
-----------------------------------
Attachment: (was: THRIFT_333_CPP_Compact_Protocol_Tests.patch)
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Fix For: 0.1
>
> Attachments: q.cpp, THRIFT_333_CPP_Compact_Protocol.patch, THRIFT_333_CPP_Compact_Protocol.v2.patch, THRIFT_333_CPP_Compact_Protocol.v3.patch, THRIFT_333_CPP_Compact_Protocol_Tests.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-333) Compact Protocol for C++
Posted by "Jérémie BORDIER (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jérémie BORDIER updated THRIFT-333:
-----------------------------------
Attachment: THRIFT_333_CPP_Compact_Protocol_Tests.patch
THRIFT_333_CPP_Compact_Protocol.v3.patch
Here's the new version, with apache headers, std::string assign optimization, along with its generic protocol test suite inspired by TCompactProtocol Java tests. Adding a new protocol only requires two lines in test/AllProtocolsTest.cpp
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Fix For: 0.1
>
> Attachments: q.cpp, THRIFT_333_CPP_Compact_Protocol.patch, THRIFT_333_CPP_Compact_Protocol.v2.patch, THRIFT_333_CPP_Compact_Protocol.v3.patch, THRIFT_333_CPP_Compact_Protocol_Tests.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-333) Compact Protocol for C++
Posted by "Jérémie BORDIER (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jérémie BORDIER updated THRIFT-333:
-----------------------------------
Attachment: THRIFT_333_CPP_Compact_Protocol_Tests.patch
Simple cosmetics..
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Fix For: 0.1
>
> Attachments: q.cpp, THRIFT_333_CPP_Compact_Protocol.patch, THRIFT_333_CPP_Compact_Protocol.v2.patch, THRIFT_333_CPP_Compact_Protocol.v3.patch, THRIFT_333_CPP_Compact_Protocol_Tests.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (THRIFT-333) Compact Protocol for C++
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
David Reiss resolved THRIFT-333.
--------------------------------
Resolution: Fixed
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Fix For: 0.1
>
> Attachments: q.cpp, t333-compact-v4.diff, THRIFT_333_CPP_Compact_Protocol.patch, THRIFT_333_CPP_Compact_Protocol.v2.patch, THRIFT_333_CPP_Compact_Protocol.v3.patch, THRIFT_333_CPP_Compact_Protocol_Tests.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-333) Compact Protocol for C++
Posted by "Jérémie BORDIER (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12688712#action_12688712 ]
Jérémie BORDIER commented on THRIFT-333:
----------------------------------------
Thanks for the hints Alexander. I'll update the patch tonight along with the tests and apache headers :-)
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Fix For: 0.1
>
> Attachments: q.cpp, THRIFT_333_CPP_Compact_Protocol.patch, THRIFT_333_CPP_Compact_Protocol.v2.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-333) Compact Protocol for C++
Posted by "Jérémie BORDIER (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jérémie BORDIER updated THRIFT-333:
-----------------------------------
Patch Info: [Patch Available]
Fix Version/s: 0.1
patch avail
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Fix For: 0.1
>
> Attachments: THRIFT_333_CPP_Compact_Protocol.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-333) Compact Protocol for C++
Posted by "Alexander Shigin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12695414#action_12695414 ]
Alexander Shigin commented on THRIFT-333:
-----------------------------------------
Oops, I've just created THRIFT-425.
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Fix For: 0.1
>
> Attachments: q.cpp, t333-compact-v4.diff, THRIFT-333_include_limits.patch, THRIFT_333_CPP_Compact_Protocol.patch, THRIFT_333_CPP_Compact_Protocol.v2.patch, THRIFT_333_CPP_Compact_Protocol.v3.patch, THRIFT_333_CPP_Compact_Protocol_Tests.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-333) Compact Protocol for C++
Posted by "Alexander Shigin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12688627#action_12688627 ]
Alexander Shigin commented on THRIFT-333:
-----------------------------------------
{code}
+uint32_t TCompactProtocol::readBinary(std::string& str) {
...
+ rsize += readVarint32(size);
+ if (size == 0)
+ return 0;
{code}
It seems the string should be nulled. TBinaryProtocol.readString null the string if gets zero size. I can't give you an example of user or generated code which depend on it right now.
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Fix For: 0.1
>
> Attachments: THRIFT_333_CPP_Compact_Protocol.patch, THRIFT_333_CPP_Compact_Protocol.v2.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-333) Compact Protocol for C++
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
David Reiss updated THRIFT-333:
-------------------------------
Attachment: t333-compact-v4.diff
Here is my version with a bunch of changes. A semi-coherent log is here:
http://gitweb.thrift-rpc.org/?p=thrift.git;a=log;h=refs/heads/pri/dreiss/cppcompact;hb=HEAD
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Fix For: 0.1
>
> Attachments: q.cpp, t333-compact-v4.diff, THRIFT_333_CPP_Compact_Protocol.patch, THRIFT_333_CPP_Compact_Protocol.v2.patch, THRIFT_333_CPP_Compact_Protocol.v3.patch, THRIFT_333_CPP_Compact_Protocol_Tests.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-333) Compact Protocol for C++
Posted by "Jérémie BORDIER (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jérémie BORDIER updated THRIFT-333:
-----------------------------------
Attachment: THRIFT_333_CPP_Compact_Protocol.v2.patch
Small refactoring, removing bytes shifting warnings on double to long conversion.
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Fix For: 0.1
>
> Attachments: THRIFT_333_CPP_Compact_Protocol.patch, THRIFT_333_CPP_Compact_Protocol.v2.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-333) Compact Protocol for C++
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12689618#action_12689618 ]
David Reiss commented on THRIFT-333:
------------------------------------
I'll give other C++ users a little longer to peruse it. I think my v4 is the front-runner at the moment.
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Fix For: 0.1
>
> Attachments: q.cpp, t333-compact-v4.diff, THRIFT_333_CPP_Compact_Protocol.patch, THRIFT_333_CPP_Compact_Protocol.v2.patch, THRIFT_333_CPP_Compact_Protocol.v3.patch, THRIFT_333_CPP_Compact_Protocol_Tests.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-333) Compact Protocol for C++
Posted by "Jérémie BORDIER (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jérémie BORDIER updated THRIFT-333:
-----------------------------------
Attachment: THRIFT-333_include_limits.patch
Missing include that breaks compilation on latest ubuntu
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Fix For: 0.1
>
> Attachments: q.cpp, t333-compact-v4.diff, THRIFT-333_include_limits.patch, THRIFT_333_CPP_Compact_Protocol.patch, THRIFT_333_CPP_Compact_Protocol.v2.patch, THRIFT_333_CPP_Compact_Protocol.v3.patch, THRIFT_333_CPP_Compact_Protocol_Tests.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-333) Compact Protocol for C++
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12689567#action_12689567 ]
Bryan Duxbury commented on THRIFT-333:
--------------------------------------
So is this ready to be committed? If so, which of the patches should be committed?
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Fix For: 0.1
>
> Attachments: q.cpp, t333-compact-v4.diff, THRIFT_333_CPP_Compact_Protocol.patch, THRIFT_333_CPP_Compact_Protocol.v2.patch, THRIFT_333_CPP_Compact_Protocol.v3.patch, THRIFT_333_CPP_Compact_Protocol_Tests.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-333) Compact Protocol for C++
Posted by "Jérémie BORDIER (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jérémie BORDIER updated THRIFT-333:
-----------------------------------
Attachment: THRIFT_333_CPP_Compact_Protocol.patch
Here's my first complete version. It's not fully tested but the tutorial passes well between Java/Cpp.
I'd like to get a review from David or Mark around the doubleToLongBits and longBitsToDouble methods, i wonder if it's not the same that the bitwise_cast<T> you're using in BinaryProtocol to counter strict aliasing issues..
Also, if anyone has advice on the best and easiest way to integrate this protocol in a "generic protocol unit test", that'd be cool :-)
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Attachments: THRIFT_333_CPP_Compact_Protocol.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-333) Compact Protocol for C++
Posted by "Alexander Shigin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-333?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alexander Shigin updated THRIFT-333:
------------------------------------
Attachment: q.cpp
I don't like an idea of temporary string in readBinary. std::string.assign works two time faster on my machine. Can anybody repeat the test?
{code}
$ g++ -Wall -O3 q.cpp
$ g++ --version
g++ (GCC) 4.2.1 20070719 [FreeBSD]
Copyright (C) 2007 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ time ./a.out | wc -l
1000
real 0m3.622s
user 0m3.611s
sys 0m0.001s
$ time ./a.out x | wc -l
1000
real 0m7.251s
user 0m3.962s
sys 0m3.269s
{code}
> Compact Protocol for C++
> ------------------------
>
> Key: THRIFT-333
> URL: https://issues.apache.org/jira/browse/THRIFT-333
> Project: Thrift
> Issue Type: Sub-task
> Components: Library (C++)
> Reporter: Bryan Duxbury
> Assignee: Jérémie BORDIER
> Priority: Trivial
> Fix For: 0.1
>
> Attachments: q.cpp, THRIFT_333_CPP_Compact_Protocol.patch, THRIFT_333_CPP_Compact_Protocol.v2.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.