Skip to content

[#1256] refactor: optimize collections contruction - #1257

Merged
roryqi merged 3 commits into
apache:masterfrom
summaryzb:empty_collection
Oct 23, 2023
Merged

[#1256] refactor: optimize collections contruction#1257
roryqi merged 3 commits into
apache:masterfrom
summaryzb:empty_collection

Conversation

@summaryzb

@summaryzb summaryzb commented Oct 23, 2023

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  1. Some empty collections are only for read, so replace it with Collections.emptyList(), Collections.emptySet(), Collections.emptyMap().
  2. In spark, writer client construct a new List prepared for shuffleBlockInfos per record, But in most case there is no blocks produced.
  3. Set proper initial value.

Why are the changes needed?

#1256

Does this PR introduce any user-facing change?

No.

How was this patch tested?

unit test

@roryqi roryqi changed the title [#1256] improvement: optimize collections contruction [#1256] refactor: optimize collections contruction Oct 23, 2023

private void sendBufferToServers(SortWriteBuffer<K, V> buffer) {
List<ShuffleBlockInfo> shuffleBlocks = Lists.newArrayList();
List<ShuffleBlockInfo> shuffleBlocks = new ArrayList<>(1);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is 1 is a reasonbale value?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's only used for adding one buffer

Set<Long> result = taskToFailedBlockIds.get(taskId);
if (result == null) {
result = Sets.newHashSet();
Collections.emptySet();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

result = Collections.emptySet()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix typo

@roryqi roryqi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @summaryzb

@roryqi
roryqi merged commit 49effee into apache:master Oct 23, 2023
@summaryzb
summaryzb deleted the empty_collection branch November 17, 2023 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants