You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@avro.apache.org by "ASF subversion and git services (Jira)" <ji...@apache.org> on 2022/11/14 08:46:00 UTC

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

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

ASF subversion and git services commented on AVRO-3657:
-------------------------------------------------------

Commit 99b0e11769062026f344ec0747aecca8bd6cb5af in avro's branch refs/heads/avro-3657-initial-buffer-size from Martin Tzvetanov Grigorov
[ https://gitbox.apache.org/repos/asf?p=avro.git;h=99b0e1176 ]

AVRO-3657: Computation of initial buffer size in OutputBuffer makes no sense

Allocate 1.25x more space than the block size

Signed-off-by: Martin Tzvetanov Grigorov <mg...@apache.org>


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