Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GLUTEN-7035][VL] Use first line of ls-remote's output as build's target commit #7036

Merged
merged 3 commits into from
Aug 28, 2024

Conversation

wForget
Copy link
Member

@wForget wForget commented Aug 27, 2024

What changes were proposed in this pull request?

Get head for target_build_commit

Fixes: #7035

How was this patch tested?

Copy link

#7035

@zhztheplayer
Copy link
Member

I feel a better fix is to use git ls-remote $VELOX_REPO refs/heads/$VELOX_BRANCH which looks up a unique ref.

@wForget
Copy link
Member Author

wForget commented Aug 27, 2024

I feel a better fix is to use git ls-remote $VELOX_REPO refs/heads/$VELOX_BRANCH which looks up a unique ref.

Thanks, this looks like a better way.

image

@wForget
Copy link
Member Author

wForget commented Aug 27, 2024

But it doesn't work for a tag

image

This reverts commit e891eff.
@zhztheplayer
Copy link
Member

But it doesn't work for a tag

image

Yes neither tag nor commit hash works. Perhaps we have to keep the original solution.

@zhztheplayer zhztheplayer changed the title [GLUTEN-7035][VL] Get head for target_build_commit [GLUTEN-7035][VL] Use first line of ls-remote's output as build's target commit Aug 28, 2024
@zhztheplayer zhztheplayer merged commit da59376 into apache:main Aug 28, 2024
42 checks passed
@GlutenPerfBot
Copy link
Contributor

===== Performance report for TPCH SF2000 with Velox backend, for reference only ====

query log/native_master_08_28_2024_time.csv log/native_master_08_26_2024_5db9de5601_time.csv difference percentage
q1 40.10 41.14 1.037 102.59%
q2 30.34 29.61 -0.726 97.61%
q3 52.02 53.63 1.604 103.08%
q4 40.89 42.24 1.346 103.29%
q5 103.89 107.54 3.649 103.51%
q6 13.28 10.83 -2.454 81.52%
q7 116.54 117.95 1.411 101.21%
q8 115.70 120.99 5.290 104.57%
q9 169.83 170.67 0.838 100.49%
q10 67.03 66.86 -0.167 99.75%
q11 27.13 26.86 -0.270 99.00%
q12 30.03 29.53 -0.495 98.35%
q13 51.89 52.01 0.126 100.24%
q14 26.74 26.47 -0.266 99.01%
q15 53.59 54.81 1.225 102.29%
q16 19.99 18.50 -1.492 92.54%
q17 130.36 129.54 -0.820 99.37%
q18 197.94 200.27 2.331 101.18%
q19 27.96 26.33 -1.633 94.16%
q20 42.29 42.35 0.058 100.14%
q21 380.46 385.88 5.426 101.43%
q22 15.92 16.01 0.092 100.58%
total 1753.91 1770.02 16.110 100.92%

sharkdtu pushed a commit to sharkdtu/gluten that referenced this pull request Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[VL] Get wrong TARGET_BUILD_COMMIT when using internal velox repository
3 participants