refactor(sparksql): Speed up sparksql compilation by splitting function registrations #11565

Yohahaha · 2024-11-18T06:18:09Z

This PR aims to speed up sparksql compilation by splitting function
registrations to multiple source files arranged according to function type.
Adds 'velox_functions_spark' for registrations and renames previous
'velox_functions_spark' as 'velox_functions_spark_impl'.

Tested the compilation time using velox_functions_spark_test target to mock
the general development process: build -> modify cpp file -> build. The
compilation time speeds up 1.5x(165s to 104s) in release mode and more in debug
mode.

Fixes #11564.

netlify · 2024-11-18T06:18:25Z

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	`fe0df08`
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/67491f5054264f0008d367d2

Yohahaha · 2024-11-18T09:45:02Z

@PHILO-HE @rui-mo @mbasmanova would you help review this PR? it's help speed up sparksql compilation time.

Yohahaha · 2024-11-20T10:49:16Z

@FelixYBW @majetideepak @pedroerp would you help review this PR? it's help speed up sparksql complication times.

rui-mo

Thanks. Added several comments.

velox/functions/sparksql/Bitwise.cpp

velox/functions/sparksql/JsonObjectKeys.h

velox/functions/sparksql/registration/Register.cpp

velox/functions/sparksql/registration/RegisterArray.cpp

velox/functions/sparksql/registration/RegisterCompare.cpp

velox/functions/sparksql/registration/RegisterDatetime.cpp

Yohahaha · 2024-11-21T07:03:14Z

@rui-mo would you help review again?

rui-mo

Thanks for the update. Added several comments.

velox/functions/sparksql/registration/Register.cpp

velox/functions/sparksql/registration/RegisterArray.cpp

velox/functions/sparksql/registration/Register.cpp

Yohahaha · 2024-11-25T12:52:02Z

@rui-mo @pedroerp @majetideepak @Yuhta would you help review again? This PR is very helpful for improving the sparksql compilation speed.

rui-mo

Thanks. Added several minors.

velox/functions/sparksql/registration/Register.cpp

rui-mo · 2024-11-26T05:44:15Z

velox/functions/sparksql/registration/Register.h

@@ -20,7 +20,7 @@

 namespace facebook::velox::functions::sparksql {

-void registerFunctions(const std::string& prefix);
+void registerFunctions(const std::string& prefix = "");


With the default value added, callers with empty string parameter like sparksql::registerFunctions("") can be simplified.

we can fix it in another PR if needed, many files will be affected.

velox/functions/sparksql/registration/RegisterMisc.cpp

velox/functions/sparksql/registration/Register.cpp

rui-mo

Thanks. Looks good to me.

rui-mo · 2024-11-27T05:57:49Z

Would you take a look if the workflow failures can be resolved after rebasing the code?

Yohahaha · 2024-11-27T11:23:19Z

Would you take a look if the workflow failures can be resolved after rebasing the code?

#11677
#11676
#11675

assignUser

Looking at the metrics this is the slowest to compile file in debug and on third place for release! https://facebookincubator.github.io/velox/bm-report/

I can see this improving dx when working on sparksql functions and reducing the need for rebuilds.

CMake looks good.

assignUser · 2024-11-30T00:11:45Z

velox/functions/sparksql/registration/CMakeLists.txt

+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+


nit: maybe add a comment explaining why this approach was taken.

Yohahaha · 2024-11-30T04:17:15Z

Looking at the metrics this is the slowest to compile file in debug and on third place for release! https://facebookincubator.github.io/velox/bm-report/

I can see this improving dx when working on sparksql functions and reducing the need for rebuilds.

CMake looks good.

thanks for clarify the compilation time improvement works, I will try improve sparksql agg/window functions compilation time in following PR.

FelixYBW · 2024-12-03T01:22:33Z

@xiaoxmeng Who can help to merge the PR? with the fix, spark functions' compile time can improve ~1.5x.

facebook-github-bot · 2024-12-03T03:08:46Z

@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-12-03T09:26:38Z

@xiaoxmeng merged this pull request in 0bb7e64.

Yohahaha · 2024-12-03T10:10:19Z

@rui-mo @FelixYBW @xiaoxmeng thank for merging!

…on registrations (facebookincubator#11565) Summary: This PR aims to speed up sparksql compilation by splitting function registrations to multiple source files arranged according to function type. Adds 'velox_functions_spark' for registrations and renames previous 'velox_functions_spark' as 'velox_functions_spark_impl'. Tested the compilation time using `velox_functions_spark_test` target to mock the general development process: build -> modify cpp file -> build. The compilation time speeds up 1.5x(165s to 104s) in release mode and more in debug mode. Fixes facebookincubator#11564. Pull Request resolved: facebookincubator#11565 Reviewed By: miaoever, kagamiori Differential Revision: D66688101 Pulled By: xiaoxmeng fbshipit-source-id: 54ba372f08c4ec91062b3d07e8e2b81aabbdef59

Yohahaha requested review from assignUser and majetideepak as code owners November 18, 2024 06:18

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 18, 2024

Yohahaha changed the title ~~Split sparksql registration code from velox_functions_spark~~ refactor(sparksql): Split sparksql registration code from velox_functions_spark Nov 18, 2024

Yohahaha changed the title ~~refactor(sparksql): Split sparksql registration code from velox_functions_spark~~ refactor(sparksql): Speed up sparksql compilation times by split sparksql registration code into serval cpp files Nov 19, 2024

rui-mo reviewed Nov 20, 2024

View reviewed changes

Yohahaha force-pushed the split-sparkfunc-register branch from 4d135d3 to e2b4294 Compare November 21, 2024 02:09

rui-mo reviewed Nov 22, 2024

View reviewed changes

Yohahaha force-pushed the split-sparkfunc-register branch from e2b4294 to d60c91d Compare November 25, 2024 06:27

rui-mo reviewed Nov 26, 2024

View reviewed changes

Yohahaha force-pushed the split-sparkfunc-register branch from fedf4a7 to 4826b23 Compare November 26, 2024 11:11

rui-mo changed the title ~~refactor(sparksql): Speed up sparksql compilation times by split sparksql registration code into serval cpp files~~ refactor(sparksql): Speed up sparksql compilation by splitting function registrations Nov 27, 2024

rui-mo approved these changes Nov 27, 2024

View reviewed changes

Yohahaha force-pushed the split-sparkfunc-register branch from 15c540d to 2f0a81d Compare November 27, 2024 07:25

Yohahaha added 5 commits November 29, 2024 09:56

fix sparksql

f4e1c19

fix format

61f40d3

fix comments

b660a55

trigger CI

8641c80

trigger CI

fe0df08

Yohahaha force-pushed the split-sparkfunc-register branch from e3d7952 to fe0df08 Compare November 29, 2024 01:56

assignUser approved these changes Nov 30, 2024

View reviewed changes

rui-mo added the ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall label Dec 2, 2024

facebook-github-bot closed this in 0bb7e64 Dec 3, 2024

facebook-github-bot added the Merged label Dec 3, 2024

Yohahaha deleted the split-sparkfunc-register branch December 3, 2024 10:10

zhouyuan mentioned this pull request Dec 4, 2024

feat: Add Spark get_json_object function #11691

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(sparksql): Speed up sparksql compilation by splitting function registrations #11565

refactor(sparksql): Speed up sparksql compilation by splitting function registrations #11565

Yohahaha commented Nov 18, 2024 •

edited by rui-mo

Loading

netlify bot commented Nov 18, 2024 •

edited

Loading

Yohahaha commented Nov 18, 2024

Yohahaha commented Nov 20, 2024

rui-mo left a comment

Yohahaha commented Nov 21, 2024

rui-mo left a comment

Yohahaha commented Nov 25, 2024

rui-mo left a comment

rui-mo Nov 26, 2024

Yohahaha Nov 26, 2024

rui-mo left a comment

rui-mo commented Nov 27, 2024

Yohahaha commented Nov 27, 2024

assignUser left a comment

assignUser Nov 30, 2024

Yohahaha commented Nov 30, 2024

FelixYBW commented Dec 3, 2024

facebook-github-bot commented Dec 3, 2024

facebook-github-bot commented Dec 3, 2024

Yohahaha commented Dec 3, 2024

refactor(sparksql): Speed up sparksql compilation by splitting function registrations #11565

refactor(sparksql): Speed up sparksql compilation by splitting function registrations #11565

Conversation

Yohahaha commented Nov 18, 2024 • edited by rui-mo Loading

netlify bot commented Nov 18, 2024 • edited Loading

✅ Deploy Preview for meta-velox canceled.

Yohahaha commented Nov 18, 2024

Yohahaha commented Nov 20, 2024

rui-mo left a comment

Choose a reason for hiding this comment

Yohahaha commented Nov 21, 2024

rui-mo left a comment

Choose a reason for hiding this comment

Yohahaha commented Nov 25, 2024

rui-mo left a comment

Choose a reason for hiding this comment

rui-mo Nov 26, 2024

Choose a reason for hiding this comment

Yohahaha Nov 26, 2024

Choose a reason for hiding this comment

rui-mo left a comment

Choose a reason for hiding this comment

rui-mo commented Nov 27, 2024

Yohahaha commented Nov 27, 2024

assignUser left a comment

Choose a reason for hiding this comment

assignUser Nov 30, 2024

Choose a reason for hiding this comment

Yohahaha commented Nov 30, 2024

FelixYBW commented Dec 3, 2024

facebook-github-bot commented Dec 3, 2024

facebook-github-bot commented Dec 3, 2024

Yohahaha commented Dec 3, 2024

Yohahaha commented Nov 18, 2024 •

edited by rui-mo

Loading

netlify bot commented Nov 18, 2024 •

edited

Loading