[jira] [Comment Edited] (AVRO-3657) Computation of initial buffer size in OutputBuffer makes no sense

["Tagir Valeev (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/AVRO-3657?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17625626#comment-17625626 ] 

Tagir Valeev edited comment on AVRO-3657 at 10/28/22 11:30 AM:
---------------------------------------------------------------

No, we don't use it. Yes, we are doing some research in static analysis field and found this case, though I don't know an existing tool which readily reports this particular code snippet. Probably we will support it in IntelliJ IDEA but I'm not sure, as existing parentheses somehow imply that the code is written as intended, and you have to look into commit history to find out that this is not the case... Please disregard the report if the code is deprecated.


was (Author: lany):
No, we don't use it. Yes, we are doing some research in static analysis field and found this case, though I don't know an existing tool which readily reports this particular code snippet. Probably we will support it in IntelliJ IDEA but I'm not sure, as existing parentheses somehow imply that the code is written as intended, and you have to look into commit history to find out that this is not the case...

> Computation of initial buffer size in OutputBuffer makes no sense
> -----------------------------------------------------------------
>
>                 Key: AVRO-3657
>                 URL: https://issues.apache.org/jira/browse/AVRO-3657
>             Project: Apache Avro
>          Issue Type: Bug
>          Components: java
>            Reporter: Tagir Valeev
>            Priority: Minor
>
> As you can see, OutputBuffer calls the superclass constructor in the following way:
> [https://github.com/apache/avro/blame/1119b6eb5b92730b27e9798793bc67f192591c15/lang/java/trevni/core/src/main/java/org/apache/trevni/OutputBuffer.java#L32]
>  
> public OutputBuffer()
> { super((BLOCK_SIZE + BLOCK_SIZE) >> 2); }
>  
> Before 3d51ba5e965561 change it was 
>  
> public OutputBuffer()
> { super(BLOCK_SIZE + BLOCK_SIZE >> 2); }
>  
> Which looked like the author did not know that >> has lower precedence than + and forgot to put parentheses. It looks like, the 3d51ba5e965561 change blindly applied static analyzer suggestions and added parentheses to preserve the current semantics. However, `(BLOCK_SIZE + BLOCK_SIZE) >> 2` is simply `BLOCK_SIZE / 2`.
>  
> My suggestion is the following:
>  * If everybody is fine with current behavior, let's update this to `super(BLOCK_SIZE / 2);` to avoid confusion.
>  * If we would like to preserve the original author's intention to allocate 1.25x more, let's place the parentheses in another way: `super(BLOCK_SIZE + (BLOCK_SIZE >> 2));`



--
This message was sent by Atlassian Jira
(v8.20.10#820010)