Fix #181: float32 accuracy issue

[willson-chen]

The issue accuracy is caused by casting from float32 to float64 when encoding, which leads to extra decimal appended to the original float. Transformation from float32 to string, and then float64 could avoid this issue.

[wongoo]

a unit test is required for it

[codecov-commenter]

Codecov Report

Merging #196 into master will increase coverage by 0.41%.
The diff coverage is 68.75%.

@@            Coverage Diff             @@
##           master     #196      +/-   ##
==========================================
+ Coverage   66.01%   66.42%   +0.41%     
==========================================
  Files          22       22              
  Lines        2845     2850       +5     
==========================================
+ Hits         1878     1893      +15     
+ Misses        749      742       -7     
+ Partials      218      215       -3     

Impacted Files	Coverage Δ
double.go	86.15% <66.66%> (-5.85%)	⬇️
encode.go	88.29% <100.00%> (ø)
decode.go	68.37% <0.00%> (+1.70%)	⬆️
string.go	57.91% <0.00%> (+2.54%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9ab1067...6eccb03. Read the comment docs.

[wongoo]

suggest to add a new func encFloat32 to receive a float32 and encode it, the string converting would degrade performance

[willson-chen]

I referenced dubbo-hessian-lite/PR#12 to create this patch.

Adding func encFloat32 is a good choice. And then we should encode the float32 directly indead of casting it to float64. BTW, casting causes the accuracy issue. But I am not sure if it will introduce any compatibility issues.

double     ::= 'D' b7 b6 b5 b4 b3 b2 b1 b0   # 64-bit float
           ::= x5b                   # 0.0
           ::= x5c                   # 1.0
           ::= x5d b0                # byte cast to double
                                     #  (-128.0 to 127.0)
           ::= x5e b1 b0             # short cast to double
           ::= x5f b3 b2 b1 b0       # 32-bit float cast to double

[wongoo]

currently hessian2 encoding all float as 64-bit float, while decoding support multiple cases.
it's worth to do effort to let the encoding follow the protocol.

[AlexStocks]

currently hessian2 encoding all float as 64-bit float, while decoding support multiple cases.
it's worth to do effort to let the encoding follow the protocol.

we can add comment here. In my opinion, we should keep pace with dubbo-hessian-lite.

[wongoo]

pls check whether dubbo-hessian-lite support 32-bit format x5f b3 b2 b1 b0 # 32-bit float cast to double

[willson-chen]

pls check whether dubbo-hessian-lite support 32-bit format x5f b3 b2 b1 b0 # 32-bit float cast to double

Hi, here is the implement of float32 encode in dubbo-hessian-lite. As we can see, dubbo-hessian-lite cast float to double. So I don't think it supports the 32-bit format.

[wongoo]

it support exactly, and fully adapt the hessian protocol, see: https://github.com/apache/dubbo-hessian-lite/blob/aea62f0a8e82cad885cc0f7f568e9c27ef6ea70c/src/main/java/com/alibaba/com/caucho/hessian/io/Hessian2Output.java#L733

[willson-chen]

@wongoo yeah, you are right. I had a careful check the code block, and foud dubbo-hessian-lite both support hessian and hessian2. hessian has only double ::= 'D' b7 b6 b5 b4 b3 b2 b1 b0 format when encoding float and double.

In fact, dubbo-hessian-lite encode the number depending the value, but not the type, float(32-bit in java) or double(64-bit in java). All the float would be transform to double, and when the value is less then 3 digits after the decimal point, the number will be encoded in x5f b3 b2 b1 b0 format.

So what should we do?

• Follow dubbo-hessian-lite, we should add the value judgement and deside which format to encode in func encFloat of double.go.
• Encode exactly depend on the type, we should add func encFloat32.

[wongoo]

Encode exactly depend on the type, we should add func encFloat32.

my opinion is adding a new func for float32.

[wongoo]

pls add unit test for math.MaxFloat32、math.MaxFloat32-1、math.MaxFloat32+1、math.MaxFloat64、math.MaxFloat64-1

[wongoo]

The command "go fmt && [[ -z git status -s ]]" exited with 1.

pls format the code

[willson-chen]

pls add unit test for math.MaxFloat32、math.MaxFloat32-1、math.MaxFloat32+1、math.MaxFloat64、math.MaxFloat64-1

Fair enough. But I need some time to adjust the test case.

pls format the code

done

[wongoo]

LGTM

[wongoo]

LGTM

[zonghaishang]

hessian2规范 float就是64位的，hessian go不应该用float32去表示它，因为可能溢出不能完全解决问题。

我的建议：

1. 针对hessian库支持，应该用float64去处理，不应该用float32，并且不应先转string，因为float是基础类型
2. 应该在readme告知开发者应该使用float64

---

Hessian2 float is 64-bit, and Hessian Go should not be represented by float32 because overflow may not solve the problem completely.

my advice:

1. Hessian Go USES Float64 encoding and decoding instead of converting a String first
2. The user should be explicitly informed in the ReadME of Hessian Go that instead of using Float32, it should be treated with Float64.

[fangyincheng]

LGTM