Skip to content

Add support for spdlog#479

Merged
eddelbuettel merged 15 commits intomasterfrom
de/sc-22999/use_spdlog
Nov 10, 2022
Merged

Add support for spdlog#479
eddelbuettel merged 15 commits intomasterfrom
de/sc-22999/use_spdlog

Conversation

@eddelbuettel
Copy link
Copy Markdown
Contributor

@eddelbuettel eddelbuettel commented Oct 28, 2022

This (draft) PR adds support for spdlog. On Windows, it is currently disabled as the linking with libspdlog.a (to support libtiledb) creates an issue.

Edit: This has been reworked, and the logging ability is now exported directly from the RcppSpdlog package. It requires a version that is slightly ahead of what CRAN has; CI is installing this explicitly right now.

Having this support allows to control logging both from the package and (via the standard config object) at the TileDB Core level:

$ r -ltiledb,RcppSpdlog -e'log_set_level("info"); set_allocation_size_preference(1024^3); \
      cfg <- tiledb_config(); \
      cfg["config.logging_level"] <- 4; \
      ctx <- tiledb_ctx(cfg); \
      print(tiledb_array("../tiledb-soma/test/soco/pbmc3k_processed/ms/mRNA/X/data", query_layout="UNORDERED")[])'
[2022-10-28 17:31:46.814] [tiledb] [Process: 1101416] [info] [tiledb_array] query: READ
[2022-10-28 17:31:46.820] [tiledb] [Process: 1101416] [info] [tiledb_array] '[' accessor started
[2022-10-28 17:31:46.945] [tiledb] [Process: 1101416] [info] [libtiledb_query_submit]
[2022-10-28 17:31:46.948] [Process: 1101416] [debug] [1666996306731388389-Context: 1] [Query: 1] [SparseGlobalOrderReader: 1] Initial data loaded
[2022-10-28 17:31:46.949] [Process: 1101416] [debug] [1666996306731388389-Context: 1] [Query: 1] [SparseGlobalOrderReader: 1] Done adding result tiles, num result tiles 49
[2022-10-28 17:31:46.949] [Process: 1101416] [debug] [1666996306731388389-Context: 1] [Query: 1] [SparseGlobalOrderReader: 1] All result tiles loaded
[2022-10-28 17:31:46.983] [Process: 1101416] [debug] [1666996306731388389-Context: 1] [Query: 1] [SparseGlobalOrderReader: 1] Done reading and unfiltering coords tiles
[2022-10-28 17:31:46.992] [Process: 1101416] [debug] [1666996306731388389-Context: 1] [Query: 1] [SparseGlobalOrderReader: 1] Done computing tile bitmaps
[2022-10-28 17:31:46.992] [Process: 1101416] [debug] [1666996306731388389-Context: 1] [Query: 1] [SparseGlobalOrderReader: 1] Done applying query condition
[2022-10-28 17:31:46.993] [Process: 1101416] [debug] [1666996306731388389-Context: 1] [Query: 1] [SparseGlobalOrderReader: 1] Done processing fragments with timestamps
[2022-10-28 17:31:47.004] [Process: 1101416] [debug] [1666996306731388389-Context: 1] [Query: 1] [SparseGlobalOrderReader: 1] Done merging result cell slabs, num slabs 49, buffers full false
[2022-10-28 17:31:47.049] [Process: 1101416] [debug] [1666996306731388389-Context: 1] [Query: 1] [SparseGlobalOrderReader: 1] Done copying tiles, buffers full false
[2022-10-28 17:31:47.050] [Process: 1101416] [debug] [1666996306731388389-Context: 1] [Query: 1] [SparseGlobalOrderReader: 1] Done with iteration, num result tiles 0
[2022-10-28 17:31:47.050] [tiledb] [Process: 1101416] [info] [libtiledb_query_status]
[2022-10-28 17:31:47.050] [tiledb] [Process: 1101416] [info] [tiledb_array] '[' query status:COMPLETE
         soma_dim_0 soma_dim_1  soma_data
      1:          0          0 -0.1714695
      2:          0          1 -0.2808120
      3:          0          2 -0.0466768
      4:          0          3 -0.4751688
      5:          0          4 -0.5440238
     ---                                 
4848640:       2637       1833 -0.1038306
4848641:       2637       1834 -0.4368798
4848642:       2637       1835 -0.0784244
4848643:       2637       1836 -0.1303272
4848644:       2637       1837 -0.4713379
$ 

The question to discuss now is whether a) this useful as-is on the two main OSs we use for development and b) whether it may be worth rejigging the core library build to export the logger (which would clearly be a more medium-term change).

@shortcut-integration
Copy link
Copy Markdown

This pull request has been linked to Shortcut Story #22999: Integrate spdlog.

@eddelbuettel eddelbuettel force-pushed the de/sc-22999/use_spdlog branch from 5e50ca4 to 699e230 Compare October 31, 2022 19:16
@eddelbuettel
Copy link
Copy Markdown
Contributor Author

Did one more round of tests and am now quite positive this works as intended -- giving us spdlog on all platforms without any double symbol woes. It currently requires RcppSpdlog 0.0.8.4 as in the ghrr drat but I will ship that to CRAN 'soon'.

@eddelbuettel eddelbuettel marked this pull request as ready for review November 1, 2022 23:16
@eddelbuettel eddelbuettel force-pushed the de/sc-22999/use_spdlog branch from a30e6f3 to 6014704 Compare November 2, 2022 23:32
@eddelbuettel eddelbuettel merged commit 5acbdd8 into master Nov 10, 2022
@eddelbuettel eddelbuettel deleted the de/sc-22999/use_spdlog branch November 10, 2022 22:21
@eddelbuettel eddelbuettel mentioned this pull request Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants