Skip to content

[SPARK-5984]Add notes and fix puzzling notes - #17073

Closed
xiaoyesoso wants to merge 1 commit into
apache:masterfrom
xiaoyesoso:patch-2
Closed

[SPARK-5984]Add notes and fix puzzling notes#17073
xiaoyesoso wants to merge 1 commit into
apache:masterfrom
xiaoyesoso:patch-2

Conversation

@xiaoyesoso

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Notes

How was this patch tested?

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

Please review http://spark.apache.org/contributing.html before opening a pull request.

@AmplabJenkins

Copy link
Copy Markdown

Can one of the admins verify this patch?

@srowen

Copy link
Copy Markdown
Member

This has no explanation. This is also copied from the Android OSS project, so I don't think I'd change the comments without reason. It'd be easier to update later if needed

@xiaoyesoso

Copy link
Copy Markdown
ContributorAuthor

@srowen
The explanations are for TimSort from Android OSS。The code has bug (see it )

privatevoidmergeCollapse() {
while (stackSize > 1) {
intn = stackSize - 2;
if (n > 0 && runLen[n-1] <= runLen[n] + runLen[n+1]) {
if (runLen[n - 1] < runLen[n + 1])
n--;
mergeAt(n);
} elseif (runLen[n] <= runLen[n + 1]) {
mergeAt(n);
} else {
break; // Invariant is established
}
}
}

But we have fixed it. (see SPARK-5984) .So leaving these explanations may puzzling.

@srowen

Copy link
Copy Markdown
Member

OK, but you're un-documenting an invariant that is still supposed to be true. Your new invariant is the same as the first too.

srowen added a commit to srowen/spark that referenced this pull request Mar 22, 2017
@srowensrowen mentioned this pull request Mar 22, 2017
zifeif2 pushed a commit to zifeif2/spark that referenced this pull request Nov 22, 2025
Closesapache#16819Closesapache#13467Closesapache#16083Closesapache#17135Closesapache#8785Closesapache#16278Closesapache#16997Closesapache#17073Closesapache#17220
Added:
Closesapache#12059Closesapache#12524Closesapache#12888Closesapache#16061
Author: Sean Owen <sowen@cloudera.com>
Closesapache#17386 from srowen/StalePRs.
Sign up for freeto 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.

3 participants

@xiaoyesoso@AmplabJenkins@srowen