search

facebookincubator / velox

A composable and fully extensible C++ execution engine library for data management systems.
https://velox-lib.io/
Apache License 2.0
3.53k stars 1.16k forks source link

fix: Move the thread local parameter from method to member parameter in HdfsReadFile #11580

Open JkSelf opened 6 days ago

JkSelf commented 6 days ago

To address this comment, we moved the thread local parameter folly::ThreadLocal<HdfsFile> file_ from a member parameter to a template parameter in the HdfsReadFile::preadInternal method. The frequent initialization of the thread local parameter in the method caused about a 5% performance degradation. We reverted the thread local parameter to a member parameter in this PR.

netlify[bot] commented 6 days ago

Deploy Preview for meta-velox canceled.

Name Link
Latest commit a273c2d758debeda3fa83c0357be7c76a3b47efc
Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/673fe032649e3d0008bd298a
JkSelf commented 6 days ago

@majetideepak Can you help to review this PR? Thanks.

majetideepak commented 3 days ago

@JkSelf changes look good. But build is failing. Can you take a look?

JkSelf commented 3 days ago

@JkSelf changes look good. But build is failing. Can you take a look?

@majetideepak Yes. Fixed the ci failing. Can you help to review again? Thanks.

JkSelf commented 2 days ago

@majetideepak It seems the failed unit test is not related with this PR. Thanks.

facebook-github-bot commented 2 days ago

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