Merge branch 'facebookincubator:main' into fix_memory

.github/workflows/linux-build.yml

@@ -16,6 +16,8 @@ name: Linux Build
 
 on:
   push:
+    branches:
+      - "main"
     paths:
       - "velox/**"
       - "!velox/docs/**"
@@ -47,6 +49,8 @@ concurrency:
 jobs:
   adapters:
     name: Linux release with adapters
+    # prevent errors when forks ff their main branch
+    if: ${{ github.repository == "facebookincubator/velox" }}
     runs-on: 8-core
     container: ghcr.io/facebookincubator/velox-dev:adapters
     defaults:
@@ -110,6 +114,8 @@ jobs:
 
   ubuntu-debug:
     runs-on: 8-core
+    # prevent errors when forks ff their main branch
+    if: ${{ github.repository == "facebookincubator/velox" }}
     name: "Ubuntu debug with resolve_dependency"
     env:
       CCACHE_DIR: "${{ github.workspace }}/.ccache"

.github/workflows/scheduled.yml

@@ -26,6 +26,19 @@ on:
       - "scripts/setup-helper-functions.sh"
       - ".github/workflows/scheduled.yml"
 
+  push:
+    branches:
+      - "main"
+    paths:
+      - "velox/**"
+      - "!velox/docs/**"
+      - "CMakeLists.txt"
+      - "CMake/**"
+      - "third_party/**"
+      - "scripts/setup-ubuntu.sh"
+      - "scripts/setup-helper-functions.sh"
+      - ".github/workflows/scheduled.yml"
+
   schedule:
     - cron: '0 3 * * *'
 
@@ -58,18 +71,20 @@ permissions:
   contents: read
 
 concurrency:
-  group: ${{ github.workflow }}-${{ github.repository }}-${{ github.head_ref || github.sha }}
+  group: ${{ github.workflow }}-${{ github.repository }}-${{ github.head_ref || github.ref }}
   cancel-in-progress: true
 
 env:
   # Run for 15 minute on PRs
-  DURATION: "${{ inputs.duration || ( github.event_name == 'pull_request' && 900 || 1800 )}}"
+  DURATION: "${{ inputs.duration || ( github.event_name != 'schedule' && 900 || 1800 )}}"
   # minimize artifact duration for PRs, keep them a bit longer for nightly runs
   RETENTION: "${{ github.event_name == 'pull_request' && 1 || 3 }}"
 
 jobs:
   compile:
     name: Build
+    # prevent errors when forks ff their main branch
+    if: ${{ github.repository == "facebookincubator/velox" }}
     runs-on: 16-core
     container: ghcr.io/facebookincubator/velox-dev:centos8
     timeout-minutes: 120
@@ -90,12 +105,26 @@ jobs:
 
     steps:
 
+      - name: Get latest commit from main
+        if: ${{ github.event_name != 'schedule' }}
+        env:
+          GH_TOKEN: ${{ github.token }}
+        id: get-head
+        run: |
+          if [ '${{ github.event_name = 'push' }}' == "true" ]; then
+            # get the parent commit of the current one to get the relevant function signatures
+            head_main=$(gh api -q '.parents.[0].sha' '/repos/facebookincubator/velox/commits/${{ github.sha }}')
+          else
+            head_main=$(gh api -H "Accept: application/vnd.github.sha" /repos/facebookincubator/velox/commits/heads/main)
+          fi
+          echo "head_main=$head_main" >> $GITHUB_OUTPUT
+
       - name: Get Function Signature Stash
         uses: assignUser/stash/restore@v1
         id: get-sig
         with:
           path: /tmp/signatures
-          key: function-signatures
+          key: function-signatures-${{ steps.get-head.outputs.head_main || github.sha }}
 
       - name: Restore ccache
         uses: assignUser/stash/restore@v1
@@ -118,15 +147,14 @@ jobs:
           mkdir -p /tmp/signatures
 
       - name: Checkout Main
-        if: ${{ steps.get-sig.outputs.stash-hit != 'true' }}
+        if: ${{ github.even_name != 'schedule' && steps.get-sig.outputs.stash-hit != 'true' }}
         uses: actions/checkout@v4
         with:
-          # hardcode ref without broken pr
-          ref: 'main'
+          ref: ${{ steps.get-head.outputs.head_main || 'main' }}
           path: velox_main
 
       - name: Build PyVelox
-        if: ${{ steps.get-sig.outputs.stash-hit != 'true' }}
+        if: ${{ github.even_name != 'schedule' && steps.get-sig.outputs.stash-hit != 'true' }}
         working-directory: velox_main
         run: |
           python3 -m venv .venv
@@ -135,7 +163,7 @@ jobs:
           make python-build
 
       - name: Create Baseline Signatures
-        if: ${{ steps.get-sig.outputs.stash-hit != 'true' }}
+        if: ${{ github.even_name != 'schedule' && steps.get-sig.outputs.stash-hit != 'true' }}
         working-directory: velox_main
         run: |
           source .venv/bin/activate
@@ -144,11 +172,11 @@ jobs:
           python3 scripts/signature.py export --presto /tmp/signatures/presto_signatures_main.json
 
       - name: Save Function Signature Stash
-        if: ${{ steps.get-sig.outputs.stash-hit != 'true' }}
+        if: ${{ github.even_name == 'pull_request' && steps.get-sig.outputs.stash-hit != 'true' }}
         uses: assignUser/stash/save@v1
         with:
           path: /tmp/signatures
-          key: function-signatures
+          key: function-signatures-${{ steps.get-head.outputs.head_main }}
 
       - name: Checkout Contender
         uses: actions/checkout@v4
@@ -172,12 +200,16 @@ jobs:
         run: ccache -s
 
       - name: Save ccache
+        # see https://github.com/actions/upload-artifact/issues/543
+        continue-on-error: true
+        if: ${{ github.even_name != 'schedule' }}
         uses: assignUser/stash/save@v1
         with:
           path: "${{ env.CCACHE_DIR }}"
           key: ccache-fuzzer-centos
 
       - name: Build PyVelox
+        if: ${{ github.even_name != 'schedule' }}
         env:
           VELOX_BUILD_DIR: "_build/debug"
         run: |
@@ -186,19 +218,40 @@ jobs:
           python3 -m pip install -e .
 
       - name: Create and test new function signatures
+        if: ${{ github.even_name != 'schedule' }}
         id: sig-check
         run: |
           source .venv/bin/activate
           python3 -m pip install deepdiff
           python3 scripts/signature.py gh_bias_check presto spark
 
       - name: Upload Signature Artifacts
+        if: ${{ github.even_name != 'schedule' }}
         uses: actions/upload-artifact@v4
         with:
           name: signatures
           path: /tmp/signatures
           retention-days: "${{ env.RETENTION }}"
 
+      - name: Prepare signatures
+        working-directory: /tmp/signatures
+        if: ${{ github.event_name == 'push' }}
+        run: |
+          # Remove irrelevant artifacts
+          rm *_bias_functions
+          rm *_signatures_main.json
+          # Rename signature files as 'main' files
+          for f in *_signatures_contender.json; do
+            mv "$f" "${f/_contender.json/_main.json}"
+          done
+
+      - name: Save Function Signature Stash
+        if: ${{ github.even_name == 'push' }}
+        uses: assignUser/stash/save@v1
+        with:
+          path: /tmp/signatures
+          key: function-signatures-${{ github.sha }}
+
       - name: Upload presto fuzzer
         uses: actions/upload-artifact@v4
         with:
@@ -267,7 +320,7 @@ jobs:
           # Run for 30 minutes instead of 15, when files relevant to presto are touched
           pr_duration: "${{ steps.changes.outputs.presto == 'true' && 1800 || 900 }}"
           # Run for 60 minutes if its a scheduled run
-          other_duration: "${{ inputs.duration || 3600 }}"
+          other_duration: "${{ inputs.duration || (github.event_name == 'push' && 1800 || 3600) }}"
           is_pr: "${{ github.event_name == 'pull_request' }}"
         run: |
 
@@ -653,6 +706,7 @@ jobs:
 
   surface-signature-errors:
     name: Signature Changes
+    if: ${{ github.event_name != 'schedule' }}
     needs: compile
     runs-on: ubuntu-latest
     steps:
@@ -667,4 +721,3 @@ jobs:
         run: |
           cat /tmp/signatures/presto_errors
           exit 1
-

CMakeLists.txt

@@ -336,8 +336,7 @@ if("${ENABLE_ALL_WARNINGS}")
          -Wno-strict-aliasing \
          -Wno-type-limits \
          -Wno-stringop-overflow \
-         -Wno-stringop-overread \
-         -Wno-return-type")
+         -Wno-stringop-overread")
   endif()
 
   set(KNOWN_WARNINGS

velox/common/base/Counters.cpp

@@ -146,7 +146,6 @@ void registerVeloxMetrics() {
   // The distribution of a root memory pool's initial capacity in range of [0,
   // 256MB] with 32 buckets. It is configured to report the capacity at P50,
   // P90, P99, and P100 percentiles.
-
   DEFINE_HISTOGRAM_METRIC(
       kMetricMemoryPoolInitialCapacityBytes,
       8L << 20,
@@ -163,6 +162,12 @@ void registerVeloxMetrics() {
   DEFINE_HISTOGRAM_METRIC(
       kMetricMemoryPoolCapacityGrowCount, 8, 0, 256, 50, 90, 99, 100);
 
+  // Tracks the count of double frees in memory allocator, indicating the
+  // possibility of buffer ownership issues when a buffer is freed more than
+  // once.
+  DEFINE_METRIC(
+      kMetricMemoryAllocatorDoubleFreeCount, facebook::velox::StatType::COUNT);
+
   /// ================== Spill related Counters =================
 
   // The number of bytes in memory to spill.

velox/common/base/Counters.h

@@ -67,6 +67,9 @@ constexpr folly::StringPiece kMetricMemoryPoolUsageLeakBytes{
 constexpr folly::StringPiece kMetricMemoryPoolReservationLeakBytes{
     "velox.memory_pool_reservation_leak_bytes"};
 
+constexpr folly::StringPiece kMetricMemoryAllocatorDoubleFreeCount{
+    "velox.memory_allocator_double_free_count"};
+
 constexpr folly::StringPiece kMetricArbitratorRequestsCount{
     "velox.arbitrator_requests_count"};
 

velox/common/file/FileSystems.h

@@ -43,6 +43,8 @@ struct FileOptions {
 
   std::unordered_map<std::string, std::string> values;
   memory::MemoryPool* pool{nullptr};
+  /// If specified then can be trusted to be the file size.
+  std::optional<int64_t> fileSize;
 };
 
 /// An abstract FileSystem

velox/common/hyperloglog/CMakeLists.txt

@@ -11,14 +11,18 @@
 # 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.
-if(${VELOX_BUILD_TESTING})
-  add_subdirectory(tests)
-endif()
-
 add_library(velox_common_hyperloglog BiasCorrection.cpp DenseHll.cpp
                                      SparseHll.cpp)
 
 target_link_libraries(
   velox_common_hyperloglog
   PUBLIC velox_memory
   PRIVATE velox_exception)
+
+if(${VELOX_BUILD_TESTING})
+  add_subdirectory(tests)
+endif()
+
+if(${VELOX_ENABLE_BENCHMARKS})
+  add_subdirectory(benchmarks)
+endif()