fix address alignment, required by cran #3415

guolinke · 2020-09-29T04:50:20Z

No description provided.

jameslamb · 2020-09-29T05:13:26Z

src/io/file_io.cpp

@@ -47,6 +47,8 @@ struct LocalFile : VirtualFileReader, VirtualFileWriter {
  }

  size_t Write(const void* buffer, size_t bytes) const {
+    // 4 bytes alignment
+    CHECK_EQ(bytes % 4, 0);


it looks like this check is failing for many of the CI jobs (and not just R)

jameslamb · 2020-09-29T05:28:37Z

Ok I tried this using the steps in #3338 (comment), got a similar error too the one showing up in CI

[LightGBM] [Info] Saving data to binary file /tmp/RtmptKlgj2/lgb.Dataset_b6968fa090e
── 1. Error: lgb.Dataset: basic construction, saving, loading (@test_dataset.R#1
[LightGBM] [Fatal] Check failed: (bytes % 4) == (0) at io/file_io.cpp, line 51 .

Backtrace:
 1. lightgbm::lgb.Dataset.save(dtest1, tmp_file)
 2. dataset$save_binary(fname)
 3. lightgbm:::lgb.call(...)

[LightGBM] [Warning] Auto-choosing row-wise multi-threading, the overhead of testing was 0.000363 seconds.
You can set `force_row_wise=true` to remove the overhead.
And if memory is not enough, you can set `force_col_wise=true`.

I have an early morning tomorrow so I'm going to get some sleep, but if you're able to fix it I can test this again tomorrow!

guolinke · 2020-09-29T07:42:00Z

@jameslamb It turns out that the fix is not trivial. many parts need to rewrite.

include/LightGBM/utils/file_io.h

jameslamb · 2020-09-29T22:48:17Z

@jameslamb It turns out that the fix is not trivial. many parts need to rewrite.

thanks very much! I'll test this right now with the steps from #3338 (comment)

jameslamb · 2020-09-29T23:20:01Z

Ok I can see that the tests are all "passing" now, but I still see misalignment errors.

Loading required package: R6
io/dataset_loader.cpp:373:76: runtime error: reference binding to misaligned address 0x55c61fb78f74 for type 'const value_type', which requires 8 byte alignment
0x55c61fb78f74: note: pointer points here
  01 00 00 00 00 00 00 00  00 00 00 00 04 00 00 00  00 00 00 00 08 00 00 00  00 00 00 00 0b 00 00 00
              ^ 
/usr/include/c++/10/bits/stl_vector.h:1198:21: runtime error: reference binding to misaligned address 0x55c61fb78f74 for type 'const long unsigned int', which requires 8 byte alignment
0x55c61fb78f74: note: pointer points here
  01 00 00 00 00 00 00 00  00 00 00 00 04 00 00 00  00 00 00 00 08 00 00 00  00 00 00 00 0b 00 00 00
              ^ 
/usr/include/c++/10/bits/vector.tcc:449:28: runtime error: reference binding to misaligned address 0x55c61fb78f74 for type 'const type', which requires 8 byte alignment
0x55c61fb78f74: note: pointer points here
  01 00 00 00 00 00 00 00  00 00 00 00 04 00 00 00  00 00 00 00 08 00 00 00  00 00 00 00 0b 00 00 00
              ^ 
/usr/include/c++/10/bits/move.h:77:36: runtime error: reference binding to misaligned address 0x55c61fb78f74 for type 'const long unsigned int', which requires 8 byte alignment
0x55c61fb78f74: note: pointer points here
  01 00 00 00 00 00 00 00  00 00 00 00 04 00 00 00  00 00 00 00 08 00 00 00  00 00 00 00 0b 00 00 00
              ^ 
/usr/include/c++/10/bits/alloc_traits.h:512:17: runtime error: reference binding to misaligned address 0x55c61fb78f74 for type 'const type', which requires 8 byte alignment
0x55c61fb78f74: note: pointer points here
  01 00 00 00 00 00 00 00  00 00 00 00 04 00 00 00  00 00 00 00 08 00 00 00  00 00 00 00 0b 00 00 00
              ^ 
/usr/include/c++/10/bits/move.h:77:36: runtime error: reference binding to misaligned address 0x55c61fb78f74 for type 'const long unsigned int', which requires 8 byte alignment
0x55c61fb78f74: note: pointer points here
  01 00 00 00 00 00 00 00  00 00 00 00 04 00 00 00  00 00 00 00 08 00 00 00  00 00 00 00 0b 00 00 00
              ^ 
/usr/include/c++/10/ext/new_allocator.h:150:46: runtime error: reference binding to misaligned address 0x55c61fb78f74 for type 'const type', which requires 8 byte alignment
0x55c61fb78f74: note: pointer points here
  01 00 00 00 00 00 00 00  00 00 00 00 04 00 00 00  00 00 00 00 08 00 00 00  00 00 00 00 0b 00 00 00
              ^ 
/usr/include/c++/10/bits/move.h:77:36: runtime error: reference binding to misaligned address 0x55c61fb78f74 for type 'const long unsigned int', which requires 8 byte alignment
0x55c61fb78f74: note: pointer points here
  01 00 00 00 00 00 00 00  00 00 00 00 04 00 00 00  00 00 00 00 08 00 00 00  00 00 00 00 0b 00 00 00
              ^ 
/usr/include/c++/10/ext/new_allocator.h:150:4: runtime error: load of misaligned address 0x55c61fb78f74 for type 'const long unsigned int', which requires 8 byte alignment
0x55c61fb78f74: note: pointer points here
  01 00 00 00 00 00 00 00  00 00 00 00 04 00 00 00  00 00 00 00 08 00 00 00  00 00 00 00 0b 00 00 00
              ^ 
/usr/include/c++/10/bits/stl_vector.h:1192:30: runtime error: reference binding to misaligned address 0x55c61fb78f8c for type 'const long unsigned int', which requires 8 byte alignment
0x55c61fb78f8c: note: pointer points here
  00 00 00 00 0b 00 00 00  00 00 00 00 0f 00 00 00  00 00 00 00 11 00 00 00  00 00 00 00 15 00 00 00
              ^ 
/usr/include/c++/10/bits/alloc_traits.h:512:17: runtime error: reference binding to misaligned address 0x55c61fb78f8c for type 'const type', which requires 8 byte alignment
0x55c61fb78f8c: note: pointer points here
  00 00 00 00 0b 00 00 00  00 00 00 00 0f 00 00 00  00 00 00 00 11 00 00 00  00 00 00 00 15 00 00 00
              ^ 
/usr/include/c++/10/bits/move.h:77:36: runtime error: reference binding to misaligned address 0x55c61fb78f8c for type 'const long unsigned int', which requires 8 byte alignment
0x55c61fb78f8c: note: pointer points here
  00 00 00 00 0b 00 00 00  00 00 00 00 0f 00 00 00  00 00 00 00 11 00 00 00  00 00 00 00 15 00 00 00
              ^ 
/usr/include/c++/10/ext/new_allocator.h:150:46: runtime error: reference binding to misaligned address 0x55c61fb78f8c for type 'const type', which requires 8 byte alignment
0x55c61fb78f8c: note: pointer points here
  00 00 00 00 0b 00 00 00  00 00 00 00 0f 00 00 00  00 00 00 00 11 00 00 00  00 00 00 00 15 00 00 00
              ^ 
/usr/include/c++/10/bits/move.h:77:36: runtime error: reference binding to misaligned address 0x55c61fb78f8c for type 'const long unsigned int', which requires 8 byte alignment
0x55c61fb78f8c: note: pointer points here
  00 00 00 00 0b 00 00 00  00 00 00 00 0f 00 00 00  00 00 00 00 11 00 00 00  00 00 00 00 15 00 00 00
              ^ 
/usr/include/c++/10/ext/new_allocator.h:150:4: runtime error: load of misaligned address 0x55c61fb78f8c for type 'const long unsigned int', which requires 8 byte alignment
0x55c61fb78f8c: note: pointer points here
  00 00 00 00 0b 00 00 00  00 00 00 00 0f 00 00 00  00 00 00 00 11 00 00 00  00 00 00 00 15 00 00 00
              ^

guolinke · 2020-09-29T23:36:15Z

Okay, they need 8 bytes alignment.

src/io/bin.cpp

guolinke · 2020-09-30T00:15:23Z

@jameslamb now it should work

jameslamb · 2020-09-30T00:43:55Z

I see this in the CI jobs

/home/runner/work/LightGBM/LightGBM/lightgbm.Rcheck/00_pkg_src/lightgbm/src/src/io/bin.cpp: In static member function ‘static int LightGBM::BinMapper::SizeForSpecificBin(int)’:
/home/runner/work/LightGBM/LightGBM/lightgbm.Rcheck/00_pkg_src/lightgbm/src/src/io/bin.cpp:525:73: error: expected ‘)’ before ‘;’ token
     size += static_cast<int>(VirtualFileWriter::AlignedSize(sizeof(int));
                                                                         ^
/home/runner/work/LightGBM/LightGBM/lightgbm.Rcheck/00_pkg_src/lightgbm/src/src/io/bin.cpp:533:82: error: expected ‘)’ before ‘;’ token
     size += static_cast<int>(VirtualFileWriter::AlignedSize(sizeof(uint32_t)) * 2;
                                                                                  ^
CMakeFiles/_lightgbm.dir/build.make:157: recipe for target 'CMakeFiles/_lightgbm.dir/src/io/bin.cpp.o' failed
make[3]: *** [CMakeFiles/_lightgbm.dir/src/io/bin.cpp.o] Error 1

src/io/bin.cpp

guolinke · 2020-09-30T00:54:38Z

sorry, it is a typo caused by editing on the web page.

jameslamb · 2020-09-30T00:59:48Z

no problem! testing right now

jameslamb · 2020-09-30T01:17:02Z

@guolinke THIS WORKED!! 🎉

No test failures, and no mis-alignment errors

guolinke · 2020-09-30T01:29:26Z

great! let us move forward to the memory leakage problem.

jameslamb · 2020-09-30T01:30:32Z

great! let us move forward to the memory leakage problem.

ok sounds good! I will approve this so we can merge it, and I'll cherry-pick it onto #3338

I'm working on trying to reproduce the CRAN valgrind checks right now.

* fix dataset binary file alignment * many fixes * fix warnings * fix bug * Update file_io.cpp * Update file_io.cpp * simplify code * Apply suggestions from code review * general * remove unneeded alignment * Update file_io.h * int32 to byte8 alignment * Apply suggestions from code review * Apply suggestions from code review

…3338) * [R-package] update DESCRIPTION per CRAN comments * newlines * Apply suggestions from code review Co-authored-by: Nikita Titov <nekit94-08@mail.ru> * more fixes * update Rbuildignore * more changes * more changes per CRAN response * add email * run examples in CI * add newest CRAN response * add Solaris patch * update patch * another attempt at ifaddrs patch * fix unnecessary comment * update configure * comments * bump version * tabs * fix address alignment, required by cran (#3415) * fix dataset binary file alignment * many fixes * fix warnings * fix bug * Update file_io.cpp * Update file_io.cpp * simplify code * Apply suggestions from code review * general * remove unneeded alignment * Update file_io.h * int32 to byte8 alignment * Apply suggestions from code review * Apply suggestions from code review * [R-package] add new copyright holder in DESCRIPTION (#3409) * [R-package] add new copyright holder in DESCRIPTION * fix role * fixing conflicts * [R-package] add new copyright holder in DESCRIPTION (#3409) * [R-package] add new copyright holder in DESCRIPTION * fix role * trying to fix conflicts * more fixes * this will work * update cran-comments * simplify solaris, add more testing docs * stuff * remove rchck docs * Apply suggestions from code review Co-authored-by: Nikita Titov <nekit94-08@mail.ru> * remove extra use of cat() * change solaris check * update docs * remove testing code * fix warning about cleanup not having execute permissions * fix cmake builds * remove blank line Co-authored-by: Nikita Titov <nekit94-08@mail.ru> Co-authored-by: Guolin Ke <guolin.ke@outlook.com>

github-actions · 2023-08-24T04:52:38Z

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

fix dataset binary file alignment

2466bf8

guolinke requested review from btrotta and chivee as code owners September 29, 2020 04:50

guolinke mentioned this pull request Sep 29, 2020

[R-package] miscellaneous changes to comply with CRAN requirements #3338

Merged

jameslamb reviewed Sep 29, 2020

View reviewed changes

guolinke added 2 commits September 29, 2020 15:35

many fixes

71c7662

fix warnings

f542125

guolinke added 4 commits September 29, 2020 16:11

fix bug

f6602de

Update file_io.cpp

c58dee3

Update file_io.cpp

3aa8455

simplify code

60c661c

guolinke commented Sep 29, 2020

View reviewed changes

include/LightGBM/utils/file_io.h Outdated Show resolved Hide resolved

guolinke commented Sep 29, 2020

View reviewed changes

include/LightGBM/utils/file_io.h Outdated Show resolved Hide resolved

guolinke added 3 commits September 29, 2020 17:44

Apply suggestions from code review

f56f468

general

dd67648

remove unneeded alignment

c68471f

jameslamb added the fix label Sep 29, 2020

guolinke added 2 commits September 30, 2020 07:38

Update file_io.h

c759c32

int32 to byte8 alignment

d6883ec

guolinke changed the title ~~fix address 4 bytes alignment, require by cran~~ fix address 8 bytes alignment, require by cran Sep 30, 2020

guolinke commented Sep 30, 2020

View reviewed changes

src/io/bin.cpp Outdated Show resolved Hide resolved

guolinke commented Sep 30, 2020

View reviewed changes

src/io/bin.cpp Outdated Show resolved Hide resolved

Apply suggestions from code review

aeba332

guolinke commented Sep 30, 2020

View reviewed changes

src/io/bin.cpp Outdated Show resolved Hide resolved

guolinke commented Sep 30, 2020

View reviewed changes

src/io/bin.cpp Outdated Show resolved Hide resolved

Apply suggestions from code review

f789a62

guolinke changed the title ~~fix address 8 bytes alignment, require by cran~~ fix address alignment, required by cran Sep 30, 2020

jameslamb self-requested a review September 30, 2020 01:30

jameslamb approved these changes Sep 30, 2020

View reviewed changes

guolinke merged commit f30dbe8 into master Sep 30, 2020

StrikerRUS deleted the fix-align branch September 30, 2020 02:10

shiyu1994 mentioned this pull request Jan 12, 2021

Trees with linear models at leaves #3299

Merged

github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix address alignment, required by cran #3415

fix address alignment, required by cran #3415

guolinke commented Sep 29, 2020

jameslamb Sep 29, 2020

jameslamb commented Sep 29, 2020

guolinke commented Sep 29, 2020

jameslamb commented Sep 29, 2020

jameslamb commented Sep 29, 2020

guolinke commented Sep 29, 2020

guolinke commented Sep 30, 2020

jameslamb commented Sep 30, 2020

guolinke commented Sep 30, 2020

jameslamb commented Sep 30, 2020

jameslamb commented Sep 30, 2020

guolinke commented Sep 30, 2020

jameslamb commented Sep 30, 2020

github-actions bot commented Aug 24, 2023

fix address alignment, required by cran #3415

fix address alignment, required by cran #3415

Conversation

guolinke commented Sep 29, 2020

jameslamb Sep 29, 2020

Choose a reason for hiding this comment

jameslamb commented Sep 29, 2020

guolinke commented Sep 29, 2020

jameslamb commented Sep 29, 2020

jameslamb commented Sep 29, 2020

guolinke commented Sep 29, 2020

guolinke commented Sep 30, 2020

jameslamb commented Sep 30, 2020

guolinke commented Sep 30, 2020

jameslamb commented Sep 30, 2020

jameslamb commented Sep 30, 2020

guolinke commented Sep 30, 2020

jameslamb commented Sep 30, 2020

github-actions bot commented Aug 24, 2023