search

oceanbase / miniob

MiniOB is a compact database that assists developers in understanding the fundamental workings of a database.
https://oceanbase.github.io/miniob/
Mulan Permissive Software License, Version 2
2.86k stars 1.01k forks source link

Update build.sh #340

Closed colommar closed 2 months ago

colommar commented 3 months ago

去掉默认自动检测当前机器上CPU的个数来决定编译并发数量，改由自己决策。

What problem were solved in this pull request?

Issue Number: close #303

Problem:收到多名同学反馈编译执行buidl.sh卡死。build.sh中会自动检测当前机器上CPU的个数来决定编译并发数量，但是很多同学的机器内存与CPU并不匹配，会导致编译卡死。

What is changed and how it works?

change MAKE_ARGS=(-j $CPU_CORES) to MAKE_ARGS=() add echo "./build.sh [BuildType] [[MakeOptions]]" in function usage add elif [[ "$arg" =~ ^-j[0-9]+ ]] then MAKE_ARGS+=("$arg") in function parse_args

  • 去掉并发编译，使用者各取所需，在执行build.sh时直接使用 -jN 参数自行选择并行度。

    Other information

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (0ec922e) 36.99% compared to head (48eb31c) 36.97%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #340 +/- ## ========================================== - Coverage 36.99% 36.97% -0.02% ========================================== Files 101 101 Lines 7272 7272 ========================================== - Hits 2690 2689 -1 - Misses 4582 4583 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

hnwyllmm commented 2 months ago

抱歉上午才开始测试，还没测试完发现你关闭了这个PR。 我测试下来整体上没有问题，可以直接使用bash build.sh -j2 这种格式来运行命令并且结果符合预期。 但是我有几点疑惑：

  1. build.sh 使用 --make 后的参数来当做make的参数，-jxxx也是make的参数，所以你的代码破坏了这个规则。如果认为这个规则不合理或不合适可以把规则一起修改掉；
  2. PR 只能接受 -j 参数不能接受 --jobs=N，看起来会比较怪。
colommar commented 2 months ago

@hnwyllmm 你好，我觉得可能是我之前的理解存在一些问题，我想要确认一下。

  1. 我认为--make这种规则很好，所以没有进行删除。同时我看到需要满足bash build.sh -j2，因此我增加了这个功能。
  2. 看到您提到想要增加--jobs=N这种参数，是直接增加bash build.sh --jobs=2这种功能，还是如何呢？
hnwyllmm commented 2 months ago

我看了下原始的issue描述，确实说的不准确。原始要解决的问题是默认的并行度会导致云主机编译卡死，最主要是需要去掉默认并行度。 参数 "-jN" 和 "--jobs=N" 是 make 命令本身支持的参数，不是 build.sh 新支持的参数。 如果你留心的话，应该注意到了build.sh并没有提供新的参数，除了 debug、release、init，参数都是尽量直接传递给 cmake 和 make命令的： build.sh [debug|release] [cmake options] --make [make options]

miniob是一个对兼容性要求很低的项目（主要为了学习），所以如果你觉得下面的格式更好，也可以： build.sh [debug|release] [make options] --cmake [cmake options] 或者你觉得有更简洁直观的方法，举个🌰：

build.sh [debug|release] --cmake=xxx --make=xxx

colommar commented 2 months ago

@hnwyllmm 好的，我理解了，我觉得如果是去掉默认并行度的话，只需要将此处改成MAKE_ARGS=()即可，不知道您认为怎么样呢？

https://github.com/oceanbase/miniob/blob/f42235649b7d3860aaecc08265bb801b12dd7ef3/build.sh#L12

hnwyllmm commented 2 months ago

我觉得这样没有问题发自我的 iPhone在 2024年2月23日，20:30，colommar @.***> 写道： @hnwyllmm 好的，我理解了，我觉得如果是去掉默认并行度的话，只需要将此处改成MAKE_ARGS=()即可，不知道您认为怎么样呢？ https://github.com/oceanbase/miniob/blob/f42235649b7d3860aaecc08265bb801b12dd7ef3/build.sh#L12

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>