You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/01/13 22:44:01 UTC
[GitHub] [arrow] ianmcook opened a new pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
ianmcook opened a new pull request #9202:
URL: https://github.com/apache/arrow/pull/9202
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#issuecomment-759794677
@github-actions crossbow submit -g r
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#discussion_r557511872
##########
File path: r/tools/linuxlibs.R
##########
@@ -412,6 +413,23 @@ with_s3_support <- function(env_vars) {
paste(env_vars, ifelse(arrow_s3, "ARROW_S3=ON", "ARROW_S3=OFF"))
}
+with_mimalloc <- function(env_vars) {
+ arrow_mimalloc <- toupper(Sys.getenv("ARROW_MIMALLOC")) == "ON" || tolower(Sys.getenv("LIBARROW_MINIMAL")) == "false"
+ if (arrow_mimalloc) {
+ # User wants mimalloc. Let's make sure they're not on gcc < 4.9
+ info <- system(paste("export", env_vars, "&& $CMAKE --system-information"), intern = TRUE)
+ info <- grep("^[A-Z_]* .*$", info, value = TRUE)
+ vals <- as.list(sub('^.*? "?(.*?)"?$', "\\1", info))
+ names(vals) <- sub("^(.*?) .*$", "\\1", info)
Review comment:
This is copy-pasted from `with_s3_support()` above. Consider factoring out into a separate function
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#issuecomment-760417349
The crossbow jobs that were failing because of gcc 4.8 are now succeeding after the latest commits. The install logs in both jobs show the following
```
**** mimalloc support not available for gcc < 4.9; building with ARROW_MIMALLOC=OFF
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#issuecomment-759834682
We have two problems here:
1) mimalloc appears not to be supported on gcc 4.8
2) the devtoolset-8 CI job that @jonkeane added appears to be using gcc 4.8 now
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#issuecomment-760406015
Revision: 6b5a193c227784cec8c0b5b3c9638a136c34d2ac
Submitted crossbow builds: [ursa-labs/crossbow @ actions-890](https://github.com/ursa-labs/crossbow/branches/all?query=actions-890)
|Task|Status|
|----|------|
|test-r-rstudio-r-base-3.6-bionic|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-890-azure-test-r-rstudio-r-base-3.6-bionic)|
|test-r-rstudio-r-base-3.6-centos7-devtoolset-8|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-890-azure-test-r-rstudio-r-base-3.6-centos7-devtoolset-8)|
|test-r-rstudio-r-base-3.6-centos8|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-890-azure-test-r-rstudio-r-base-3.6-centos8)|
|test-r-rstudio-r-base-3.6-opensuse15|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-890-azure-test-r-rstudio-r-base-3.6-opensuse15)|
|test-r-rstudio-r-base-3.6-opensuse42|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-890-azure-test-r-rstudio-r-base-3.6-opensuse42)|
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#issuecomment-759796369
Revision: 3ef71b5be608919752618395876f584fe78c7297
Submitted crossbow builds: [ursa-labs/crossbow @ actions-887](https://github.com/ursa-labs/crossbow/branches/all?query=actions-887)
|Task|Status|
|----|------|
|conda-linux-gcc-py36-cpu-r36|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-887-azure-conda-linux-gcc-py36-cpu-r36)|
|conda-linux-gcc-py37-cpu-r40|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-887-azure-conda-linux-gcc-py37-cpu-r40)|
|conda-osx-clang-py36-r36|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-887-azure-conda-osx-clang-py36-r36)|
|conda-osx-clang-py37-r40|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-887-azure-conda-osx-clang-py37-r40)|
|conda-win-vs2017-py36-r36|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-887-azure-conda-win-vs2017-py36-r36)|
|conda-win-vs2017-py37-r40|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-887-azure-conda-win-vs2017-py37-r40)|
|homebrew-r-autobrew|[](https://travis-ci.org/ursa-labs/crossbow/branches)|
|test-r-linux-as-cran|[](https://github.com/ursa-labs/crossbow/actions?query=branch:actions-887-github-test-r-linux-as-cran)|
|test-r-rhub-ubuntu-gcc-release|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-887-azure-test-r-rhub-ubuntu-gcc-release)|
|test-r-rocker-r-base-latest|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-887-azure-test-r-rocker-r-base-latest)|
|test-r-rstudio-r-base-3.6-bionic|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-887-azure-test-r-rstudio-r-base-3.6-bionic)|
|test-r-rstudio-r-base-3.6-centos7-devtoolset-8|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-887-azure-test-r-rstudio-r-base-3.6-centos7-devtoolset-8)|
|test-r-rstudio-r-base-3.6-centos8|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-887-azure-test-r-rstudio-r-base-3.6-centos8)|
|test-r-rstudio-r-base-3.6-opensuse15|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-887-azure-test-r-rstudio-r-base-3.6-opensuse15)|
|test-r-rstudio-r-base-3.6-opensuse42|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-887-azure-test-r-rstudio-r-base-3.6-opensuse42)|
|test-r-version-compatibility|[](https://github.com/ursa-labs/crossbow/actions?query=branch:actions-887-github-test-r-version-compatibility)|
|test-r-versions|[](https://github.com/ursa-labs/crossbow/actions?query=branch:actions-887-github-test-r-versions)|
|test-ubuntu-18.04-r-sanitizer|[](https://dev.azure.com/ursa-labs/crossbow/_build/latest?definitionId=1&branchName=actions-887-azure-test-ubuntu-18.04-r-sanitizer)|
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#issuecomment-760419019
Great, can you (1) remove [WIP] from the issue title and then (2) resolve the merge conflict, which will also trigger CI.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson closed pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging
Posted by GitBox <gi...@apache.org>.
nealrichardson closed pull request #9202:
URL: https://github.com/apache/arrow/pull/9202
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on a change in pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#discussion_r556929562
##########
File path: r/README.md
##########
@@ -103,6 +103,7 @@ cmake
-DARROW_DATASET=ON \
-DARROW_FILESYSTEM=ON \
-DARROW_JEMALLOC=ON \
+ -DARROW_MIMALLOC=ON \
Review comment:
Maybe list this as optional below
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
jonkeane commented on pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#issuecomment-759840208
I'm looking into the devtoolset-8 issue, and sure enough at _one point_ in that PR it was successful in using gcc 8.3, but the final runs / commits that were merged appear to revert back to using 4.8. It looks like the change that borked things was https://github.com/apache/arrow/pull/8932/commits/4643401d43b880789962e47e50e0576e341437ef. You could try reverting that to see if the devtools-8 crossbow builds are using gcc 8.3 without it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#issuecomment-760305649
@github-actions crossbow submit test-r-rstudio-r-base-3.6-*
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on a change in pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#discussion_r557523649
##########
File path: r/tools/linuxlibs.R
##########
@@ -412,6 +413,23 @@ with_s3_support <- function(env_vars) {
paste(env_vars, ifelse(arrow_s3, "ARROW_S3=ON", "ARROW_S3=OFF"))
}
+with_mimalloc <- function(env_vars) {
+ arrow_mimalloc <- toupper(Sys.getenv("ARROW_MIMALLOC")) == "ON" || tolower(Sys.getenv("LIBARROW_MINIMAL")) == "false"
+ if (arrow_mimalloc) {
+ # User wants mimalloc. Let's make sure they're not on gcc < 4.9
+ info <- system(paste("export", env_vars, "&& $CMAKE --system-information"), intern = TRUE)
+ info <- grep("^[A-Z_]* .*$", info, value = TRUE)
+ vals <- as.list(sub('^.*? "?(.*?)"?$', "\\1", info))
+ names(vals) <- sub("^(.*?) .*$", "\\1", info)
Review comment:
Yes, let's factor out the gcc 4.9 check to a function
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#issuecomment-760339590
mimalloc is pure C, but it probably needs some system calls that are not present on old glibcs or kernels.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#issuecomment-760340332
Ah my bad. It's specifically a gcc issue fixed in 4.9: https://stackoverflow.com/questions/20326604/stdatomic-h-in-gcc-4-8
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson edited a comment on pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
nealrichardson edited a comment on pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#issuecomment-759834682
We have two problems here:
1) mimalloc appears not to be supported on gcc 4.8 (we can check for this in linuxlibs.R and turn off mimalloc, like we already do for ARROW_S3)
2) the devtoolset-8 CI job that @jonkeane added appears to be using gcc 4.8 now
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#issuecomment-760307992
> Do we know what minimum GCC version mimalloc requires?
My guess is that it needs full C++11 support, and so that just means gcc > 4.8, so I think this is fine.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#issuecomment-760271412
Do we know what minimum GCC version mimalloc requires? I dug around in the mimalloc repo and various other places but couldn't find any indication of that
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#discussion_r557609508
##########
File path: r/tools/linuxlibs.R
##########
@@ -412,6 +413,23 @@ with_s3_support <- function(env_vars) {
paste(env_vars, ifelse(arrow_s3, "ARROW_S3=ON", "ARROW_S3=OFF"))
}
+with_mimalloc <- function(env_vars) {
+ arrow_mimalloc <- toupper(Sys.getenv("ARROW_MIMALLOC")) == "ON" || tolower(Sys.getenv("LIBARROW_MINIMAL")) == "false"
+ if (arrow_mimalloc) {
+ # User wants mimalloc. Let's make sure they're not on gcc < 4.9
+ info <- system(paste("export", env_vars, "&& $CMAKE --system-information"), intern = TRUE)
+ info <- grep("^[A-Z_]* .*$", info, value = TRUE)
+ vals <- as.list(sub('^.*? "?(.*?)"?$', "\\1", info))
+ names(vals) <- sub("^(.*?) .*$", "\\1", info)
Review comment:
Factored out in 6b5a193
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#issuecomment-759785270
https://issues.apache.org/jira/browse/ARROW-11240
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#discussion_r557543610
##########
File path: r/README.md
##########
@@ -103,6 +103,7 @@ cmake
-DARROW_DATASET=ON \
-DARROW_FILESYSTEM=ON \
-DARROW_JEMALLOC=ON \
+ -DARROW_MIMALLOC=ON \
Review comment:
Done in a59b9a2. I noticed the S3 flag wasn't in the list of optional flags here so I added that as well
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #9202: ARROW-11240: [Packaging][R] Add mimalloc to R packaging [WIP]
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #9202:
URL: https://github.com/apache/arrow/pull/9202#issuecomment-759855172
👍 that can be done on a separate issue, don't need to block this. gcc 4.8 workaround is still needed for the other build.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org