[jira] [Commented] (AVRO-1641) parser.java stack should expand quickly up to some threshold rather than start at the threshold
[
https://issues.apache.org/jira/browse/AVRO-1641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15042226#comment-15042226
]
Zoltan Farkas commented on AVRO-1641:
-
I actually changed the logic in my fork as follows:
this.stack = new Symbol[10];
+
growth strategy similar to java.util.ArrayList. see:
https://github.com/zolyfarkas/avro/blob/trunk/lang/java/avro/src/main/java/org/apache/avro/io/parsing/Parser.java#L65
In any case the initial size of 5 is too small and results in lots of
expansions... The currently expansion logic implementation is inefficient, so I
completely redone it in my fork.
> parser.java stack should expand quickly up to some threshold rather than
> start at the threshold
> ---
>
> Key: AVRO-1641
> URL: https://issues.apache.org/jira/browse/AVRO-1641
> Project: Avro
> Issue Type: Bug
>Affects Versions: 1.7.7, 1.8.0
>Reporter: Zoltan Farkas
>Assignee: Zoltan Farkas
>Priority: Minor
> Attachments: AVRO-1641.patch
>
>
> at Parser.java line 65
> (https://github.com/apache/avro/blob/trunk/lang/java/avro/src/main/java/org/apache/avro/io/parsing/Parser.java#L65):
>
> {noformat}
> private void expandStack() {
> stack = Arrays.copyOf(stack, stack.length+Math.max(stack.length,1024));
> }
> {noformat}
> should probably be:
> {noformat}
> private void expandStack() {
> stack = Arrays.copyOf(stack, stack.length+Math.min(stack.length,1024));
> }
> {noformat}
> This expansion probably is intended to grow exponentially up to 1024, and not
> exponentially after 1024...
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (AVRO-1641) parser.java stack should expand quickly up to some threshold rather than start at the threshold
[
https://issues.apache.org/jira/browse/AVRO-1641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15025267#comment-15025267
]
Zoltan Farkas commented on AVRO-1641:
-
Hi Sean, as part of this jira we might want to also review the initial stack
size, which is currently:
{noformat}
this.stack = new Symbol[5]; // Start small to make sure expansion code works
{noformat}
The comment does not inspire much confidence... The initial size should be
dictated by what is the right size (to minimize nr of expansions/memory usage)
for most cases.
The comment implies that the original size is small on purpose to trigger an
expansion, which is quite a bad idea...
> parser.java stack should expand quickly up to some threshold rather than
> start at the threshold
> ---
>
> Key: AVRO-1641
> URL: https://issues.apache.org/jira/browse/AVRO-1641
> Project: Avro
> Issue Type: Bug
>Affects Versions: 1.7.7, 1.8.0
>Reporter: Zoltan Farkas
>Assignee: Zoltan Farkas
>Priority: Minor
> Attachments: AVRO-1641.patch
>
>
> at Parser.java line 65
> (https://github.com/apache/avro/blob/trunk/lang/java/avro/src/main/java/org/apache/avro/io/parsing/Parser.java#L65):
>
> private void expandStack() {
> stack = Arrays.copyOf(stack, stack.length+Math.max(stack.length,1024));
> }
> should probably be:
> private void expandStack() {
> stack = Arrays.copyOf(stack, stack.length+Math.min(stack.length,1024));
> }
> This expansion probably is intended to grow exponentially up to 1024, and not
> exponentially after 1024...
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (AVRO-1641) parser.java stack should expand quickly up to some threshold rather than start at the threshold
[
https://issues.apache.org/jira/browse/AVRO-1641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15024562#comment-15024562
]
Zoltan Farkas commented on AVRO-1641:
-
Just be careful with the benchmarking process,
I often see devs use naive benchmarks which show some result that has nothing
to do with the reality.
I do not trust any benchmark which is not done with a benchmarking framework
like JMH...
Here is how I use JMH in my project:
https://github.com/zolyfarkas/spf4j/tree/master/spf4j-benchmarks
A good benchmark has a good amount of multithreaded runs... (large allocations
outside of TLAB kill scalability due to the locking overhead incurred.)
> parser.java stack should expand quickly up to some threshold rather than
> start at the threshold
> ---
>
> Key: AVRO-1641
> URL: https://issues.apache.org/jira/browse/AVRO-1641
> Project: Avro
> Issue Type: Bug
>Affects Versions: 1.7.7, 1.8.0
>Reporter: Zoltan Farkas
>Assignee: Zoltan Farkas
>Priority: Minor
> Attachments: AVRO-1641.patch
>
>
> at Parser.java line 65
> (https://github.com/apache/avro/blob/trunk/lang/java/avro/src/main/java/org/apache/avro/io/parsing/Parser.java#L65):
>
> private void expandStack() {
> stack = Arrays.copyOf(stack, stack.length+Math.max(stack.length,1024));
> }
> should probably be:
> private void expandStack() {
> stack = Arrays.copyOf(stack, stack.length+Math.min(stack.length,1024));
> }
> This expansion probably is intended to grow exponentially up to 1024, and not
> exponentially after 1024...
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (AVRO-1641) parser.java stack should expand quickly up to some threshold rather than start at the threshold
[
https://issues.apache.org/jira/browse/AVRO-1641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15022895#comment-15022895
]
Sean Busbey commented on AVRO-1641:
---
FYI, I started prepping to put this in, but my first pass at a simple benchmark
showed things getting worse. So I'll need to make something more formal to show
the impact of hte change.
> parser.java stack should expand quickly up to some threshold rather than
> start at the threshold
> ---
>
> Key: AVRO-1641
> URL: https://issues.apache.org/jira/browse/AVRO-1641
> Project: Avro
> Issue Type: Bug
>Affects Versions: 1.7.7, 1.8.0
>Reporter: Zoltan Farkas
>Assignee: Zoltan Farkas
>Priority: Minor
> Attachments: AVRO-1641.patch
>
>
> at Parser.java line 65
> (https://github.com/apache/avro/blob/trunk/lang/java/avro/src/main/java/org/apache/avro/io/parsing/Parser.java#L65):
>
> private void expandStack() {
> stack = Arrays.copyOf(stack, stack.length+Math.max(stack.length,1024));
> }
> should probably be:
> private void expandStack() {
> stack = Arrays.copyOf(stack, stack.length+Math.min(stack.length,1024));
> }
> This expansion probably is intended to grow exponentially up to 1024, and not
> exponentially after 1024...
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (AVRO-1641) parser.java stack should expand quickly up to some threshold rather than start at the threshold
[
https://issues.apache.org/jira/browse/AVRO-1641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15014233#comment-15014233
]
Sean Busbey commented on AVRO-1641:
---
I suspect the limits of our current developer bandwidth is the thing preventing
such folks from picking up these kinds of low hanging fruit and posting
patches. It would definitely be nice if we could find the time to grab these
kinds of issues.
> parser.java stack should expand quickly up to some threshold rather than
> start at the threshold
> ---
>
> Key: AVRO-1641
> URL: https://issues.apache.org/jira/browse/AVRO-1641
> Project: Avro
> Issue Type: Bug
>Affects Versions: 1.7.7, 1.8.0
>Reporter: Zoltan Farkas
>Priority: Minor
> Attachments: AVRO-1641.patch
>
>
> at Parser.java line 65
> (https://github.com/apache/avro/blob/trunk/lang/java/avro/src/main/java/org/apache/avro/io/parsing/Parser.java#L65):
>
> private void expandStack() {
> stack = Arrays.copyOf(stack, stack.length+Math.max(stack.length,1024));
> }
> should probably be:
> private void expandStack() {
> stack = Arrays.copyOf(stack, stack.length+Math.min(stack.length,1024));
> }
> This expansion probably is intended to grow exponentially up to 1024, and not
> exponentially after 1024...
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (AVRO-1641) parser.java stack should expand quickly up to some threshold rather than start at the threshold
[
https://issues.apache.org/jira/browse/AVRO-1641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15014109#comment-15014109
]
Zoltan Farkas commented on AVRO-1641:
-
For a simple 1-3 line change somebody with privileges should just go ahead make
the change and commit it... that's what I do in my projects...
> parser.java stack should expand quickly up to some threshold rather than
> start at the threshold
> ---
>
> Key: AVRO-1641
> URL: https://issues.apache.org/jira/browse/AVRO-1641
> Project: Avro
> Issue Type: Bug
>Affects Versions: 1.7.7, 1.8.0
>Reporter: Zoltan Farkas
>Priority: Minor
> Attachments: AVRO-1641.patch
>
>
> at Parser.java line 65
> (https://github.com/apache/avro/blob/trunk/lang/java/avro/src/main/java/org/apache/avro/io/parsing/Parser.java#L65):
>
> private void expandStack() {
> stack = Arrays.copyOf(stack, stack.length+Math.max(stack.length,1024));
> }
> should probably be:
> private void expandStack() {
> stack = Arrays.copyOf(stack, stack.length+Math.min(stack.length,1024));
> }
> This expansion probably is intended to grow exponentially up to 1024, and not
> exponentially after 1024...
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
