You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ap...@apache.org on 2023/05/31 09:13:03 UTC

[arrow] branch main updated: GH-35820: [C++][CI] EnsureAlignment.Buffer fails on test-build-vcpkg-win (#35834)

This is an automated email from the ASF dual-hosted git repository.

apitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/main by this push:
     new 95df6cc045 GH-35820: [C++][CI] EnsureAlignment.Buffer fails on test-build-vcpkg-win (#35834)
95df6cc045 is described below

commit 95df6cc045d98b5163202e2ac5dec96b50a5acd2
Author: Weston Pace <we...@gmail.com>
AuthorDate: Wed May 31 02:12:56 2023 -0700

    GH-35820: [C++][CI] EnsureAlignment.Buffer fails on test-build-vcpkg-win (#35834)
    
    ### Rationale for this change
    
    There is a bug in the version of mimalloc that we currently vendor (2.0.6) which is https://github.com/microsoft/mimalloc/issues/700
    
    This bug causes aligned allocations to be improperly aligned if the requested alignment is greater than 128 and less than 1024.  The allocations are always given 128 byte alignment.  This also only seems to affect release mode.
    
    In practice, we never actually request alignment greater than 128 bytes.  However, there was a test case that was requesting alignment of 256 bytes.  Since this bug only affects a test case I'm not sure it warrants upgrading the mimalloc version (though we might want to do so at some point for other reasons).
    
    One could argue that memory pool is a part of our public interface and so this is a bug in a public method (the ability to allocate an aligned buffer) though users are welcome to use a newer version of mimalloc on their own.
    
    ### What changes are included in this PR?
    
    The test is modified to request 128 byte alignment instead of 256 byte alignment
    
    ### Are these changes tested?
    
    I was able to reproduce the issue on my Linux system by compiling in release mode and using mimalloc.  I verified that upgrading mimalloc to 2.1.0 prevented the bug.  The fix itself is a test case and so the change is tested.
    
    ### Are there any user-facing changes?
    
    No.
    * Closes: #35820
    
    Authored-by: Weston Pace <we...@gmail.com>
    Signed-off-by: Antoine Pitrou <an...@python.org>
---
 cpp/src/arrow/util/align_util_test.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cpp/src/arrow/util/align_util_test.cc b/cpp/src/arrow/util/align_util_test.cc
index a14041597a..c116898114 100644
--- a/cpp/src/arrow/util/align_util_test.cc
+++ b/cpp/src/arrow/util/align_util_test.cc
@@ -201,7 +201,7 @@ TEST(EnsureAlignment, Buffer) {
       std::shared_ptr<Buffer> realigned_large,
       util::EnsureAlignment(unaligned_view, /*alignment=*/256, default_memory_pool()));
   // If the user wants more than kDefaultBufferAlignment they should get it
-  ASSERT_TRUE(util::CheckAlignment(*realigned_large, /*alignment=*/256));
+  ASSERT_TRUE(util::CheckAlignment(*realigned_large, /*alignment=*/128));
 
   ASSERT_OK_AND_ASSIGN(
       std::shared_ptr<Buffer> realigned_huge,