You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fury.apache.org by "LiangliangSui (via GitHub)" <gi...@apache.org> on 2024/05/08 02:43:13 UTC

[PR] chore: Fury header add language field [incubator-fury]

LiangliangSui opened a new pull request, #1612:
URL: https://github.com/apache/incubator-fury/pull/1612

   ## What does this PR do?
   Add serialization language field for Fury Header in xlang_spec.
   
   ## Related issues
   
   https://github.com/apache/incubator-fury/issues/1607
   
   
   ## Does this PR introduce any user-facing change?
   
   
   - [ ] Does this PR introduce any public API change?
   - [x] Does this PR introduce any binary protocol compatibility change?
   
   
   ## Benchmark
   
   <!--
   When the PR has an impact on performance (if you don't know whether the PR will have an impact on performance, you can submit the PR first, and if it will have impact on performance, the code reviewer will explain it), be sure to attach a benchmark data here.
   -->
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] chore: Fury header add language field [incubator-fury]

Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on code in PR #1612:
URL: https://github.com/apache/incubator-fury/pull/1612#discussion_r1594257470


##########
docs/specification/xlang_serialization_spec.md:
##########
@@ -139,16 +139,17 @@ Fury will write the byte order for that object into the data instead of converti
 Fury header consists starts one byte:
 
 ```
-|    2 bytes   |     4 bits    | 1 bit | 1 bit | 1 bit  | 1 bit |          optional 4 bytes          |
-+--------------+---------------+-------+-------+--------+-------+------------------------------------+
-| magic number | reserved bits |  oob  | xlang | endian | null  | unsigned int for meta start offset |
+|    2 bytes   |     4 bits    | 1 bit | 1 bit | 1 bit  | 1 bit |   1 byte   |          optional 4 bytes          |
++--------------+---------------+-------+-------+--------+-------+------------+------------------------------------+
+| magic number | reserved bits |  oob  | xlang | endian | null  |  language  | unsigned int for meta start offset |
 ```
 
 - magic number: used to identify fury serialization protocol, current version use `0x62d4`.
 - null flag: 1 when object is null, 0 otherwise. If an object is null, other bits won't be set.
 - endian flag: 1 when data is encoded by little endian, 0 for big endian.
 - xlang flag: 1 when serialization uses xlang format, 0 when serialization uses Fury java format.
 - oob flag: 1 when passed `BufferCallback` is not null, 0 otherwise.
+- language: the language when serializing objects, such as JAVA, PYTHON, GO, etc.

Review Comment:
   Done



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] chore: Fury header add language field [incubator-fury]

Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #1612:
URL: https://github.com/apache/incubator-fury/pull/1612#discussion_r1594142048


##########
docs/specification/xlang_serialization_spec.md:
##########
@@ -139,16 +139,17 @@ Fury will write the byte order for that object into the data instead of converti
 Fury header consists starts one byte:
 
 ```
-|    2 bytes   |     4 bits    | 1 bit | 1 bit | 1 bit  | 1 bit |          optional 4 bytes          |
-+--------------+---------------+-------+-------+--------+-------+------------------------------------+
-| magic number | reserved bits |  oob  | xlang | endian | null  | unsigned int for meta start offset |
+|    2 bytes   |     4 bits    | 1 bit | 1 bit | 1 bit  | 1 bit |         1 byte           |          optional 4 bytes          |
++--------------+---------------+-------+-------+--------+-------+--------------------------+------------------------------------+
+| magic number | reserved bits |  oob  | xlang | endian | null  |  serialization language  | unsigned int for meta start offset |

Review Comment:
   `serialization language` looks a little weird, how about removing serialization



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] chore: Fury header add language field [incubator-fury]

Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #1612:
URL: https://github.com/apache/incubator-fury/pull/1612#discussion_r1594139541


##########
docs/specification/xlang_serialization_spec.md:
##########
@@ -139,16 +139,17 @@ Fury will write the byte order for that object into the data instead of converti
 Fury header consists starts one byte:
 
 ```
-|    2 bytes   |     4 bits    | 1 bit | 1 bit | 1 bit  | 1 bit |          optional 4 bytes          |
-+--------------+---------------+-------+-------+--------+-------+------------------------------------+
-| magic number | reserved bits |  oob  | xlang | endian | null  | unsigned int for meta start offset |
+|    2 bytes   |     4 bits    | 1 bit | 1 bit | 1 bit  | 1 bit |         1 byte           |          optional 4 bytes          |
++--------------+---------------+-------+-------+--------+-------+--------------------------+------------------------------------+
+| magic number | reserved bits |  oob  | xlang | endian | null  |  serialization language  | unsigned int for meta start offset |
 ```
 
 - magic number: used to identify fury serialization protocol, current version use `0x62d4`.
 - null flag: 1 when object is null, 0 otherwise. If an object is null, other bits won't be set.
 - endian flag: 1 when data is encoded by little endian, 0 for big endian.
 - xlang flag: 1 when serialization uses xlang format, 0 when serialization uses Fury java format.
 - oob flag: 1 when passed `BufferCallback` is not null, 0 otherwise.
+- serialization language: indicates the language used for serialization, such as JAVA, PYTHON, GO, etc.

Review Comment:
   How about:
   ```suggestion
   - language: the language when serializing objects, such as JAVA, PYTHON, GO, etc.
   ```



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] chore: Fury header add language field [incubator-fury]

Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on code in PR #1612:
URL: https://github.com/apache/incubator-fury/pull/1612#discussion_r1594151766


##########
docs/specification/xlang_serialization_spec.md:
##########
@@ -139,16 +139,17 @@ Fury will write the byte order for that object into the data instead of converti
 Fury header consists starts one byte:
 
 ```
-|    2 bytes   |     4 bits    | 1 bit | 1 bit | 1 bit  | 1 bit |          optional 4 bytes          |
-+--------------+---------------+-------+-------+--------+-------+------------------------------------+
-| magic number | reserved bits |  oob  | xlang | endian | null  | unsigned int for meta start offset |
+|    2 bytes   |     4 bits    | 1 bit | 1 bit | 1 bit  | 1 bit |         1 byte           |          optional 4 bytes          |
++--------------+---------------+-------+-------+--------+-------+--------------------------+------------------------------------+
+| magic number | reserved bits |  oob  | xlang | endian | null  |  serialization language  | unsigned int for meta start offset |

Review Comment:
   Updated



##########
docs/specification/xlang_serialization_spec.md:
##########
@@ -139,16 +139,17 @@ Fury will write the byte order for that object into the data instead of converti
 Fury header consists starts one byte:
 
 ```
-|    2 bytes   |     4 bits    | 1 bit | 1 bit | 1 bit  | 1 bit |          optional 4 bytes          |
-+--------------+---------------+-------+-------+--------+-------+------------------------------------+
-| magic number | reserved bits |  oob  | xlang | endian | null  | unsigned int for meta start offset |
+|    2 bytes   |     4 bits    | 1 bit | 1 bit | 1 bit  | 1 bit |         1 byte           |          optional 4 bytes          |
++--------------+---------------+-------+-------+--------+-------+--------------------------+------------------------------------+
+| magic number | reserved bits |  oob  | xlang | endian | null  |  serialization language  | unsigned int for meta start offset |
 ```
 
 - magic number: used to identify fury serialization protocol, current version use `0x62d4`.
 - null flag: 1 when object is null, 0 otherwise. If an object is null, other bits won't be set.
 - endian flag: 1 when data is encoded by little endian, 0 for big endian.
 - xlang flag: 1 when serialization uses xlang format, 0 when serialization uses Fury java format.
 - oob flag: 1 when passed `BufferCallback` is not null, 0 otherwise.
+- serialization language: indicates the language used for serialization, such as JAVA, PYTHON, GO, etc.

Review Comment:
   Updated



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] chore: Fury header add language field [incubator-fury]

Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #1612:
URL: https://github.com/apache/incubator-fury/pull/1612#discussion_r1594218760


##########
docs/specification/xlang_serialization_spec.md:
##########
@@ -139,16 +139,17 @@ Fury will write the byte order for that object into the data instead of converti
 Fury header consists starts one byte:
 
 ```
-|    2 bytes   |     4 bits    | 1 bit | 1 bit | 1 bit  | 1 bit |          optional 4 bytes          |
-+--------------+---------------+-------+-------+--------+-------+------------------------------------+
-| magic number | reserved bits |  oob  | xlang | endian | null  | unsigned int for meta start offset |
+|    2 bytes   |     4 bits    | 1 bit | 1 bit | 1 bit  | 1 bit |   1 byte   |          optional 4 bytes          |
++--------------+---------------+-------+-------+--------+-------+------------+------------------------------------+
+| magic number | reserved bits |  oob  | xlang | endian | null  |  language  | unsigned int for meta start offset |
 ```
 
 - magic number: used to identify fury serialization protocol, current version use `0x62d4`.
 - null flag: 1 when object is null, 0 otherwise. If an object is null, other bits won't be set.
 - endian flag: 1 when data is encoded by little endian, 0 for big endian.
 - xlang flag: 1 when serialization uses xlang format, 0 when serialization uses Fury java format.
 - oob flag: 1 when passed `BufferCallback` is not null, 0 otherwise.
+- language: the language when serializing objects, such as JAVA, PYTHON, GO, etc.

Review Comment:
   Could we add a note that Fury can use this flag to determine whether spend more time on serialization to make the deserialization faster for dynamic languages.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] chore: Fury header add language field [incubator-fury]

Posted by "LiangliangSui (via GitHub)" <gi...@apache.org>.
LiangliangSui commented on PR #1612:
URL: https://github.com/apache/incubator-fury/pull/1612#issuecomment-2099630106

   PTAL @chaokunyang @theweipeng 


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org


Re: [PR] chore: Fury header add language field [incubator-fury]

Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang merged PR #1612:
URL: https://github.com/apache/incubator-fury/pull/1612


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fury.apache.org
For additional commands, e-mail: commits-help@fury.apache.org