Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix model locale issue and improve model R/W performance. #3405

Merged
merged 33 commits into from
Dec 8, 2020
Merged

Fix model locale issue and improve model R/W performance. #3405

merged 33 commits into from
Dec 8, 2020

Commits on Nov 24, 2020

  1. Fix LightGBM models locale sensitivity and improve R/W performance. 

    When Java is used, the default C++ locale is broken. This is true for
Java providers that use the C API or even Python models that require JEP.

This patch solves that issue making the model reads/writes insensitive
to such settings.
To achieve it, within the model read/write codebase:
 - C++ streams are imbued with the classic locale
 - Calls to functions that are dependent on the locale are replaced
 - The default locale is not changed!

This approach means:
 - The user's locale is never tampered with, avoiding issues such as
    microsoft#2979 with the previous
    approach microsoft#2891
 - Datasets can still be read according the user's locale
 - The model file has a single format independent of locale

Changes:
 - Add CommonC namespace which provides faster locale-independent versions of Common's methods
 - Model code makes conversions through CommonC
 - Cleanup unused Common methods
 - Performance improvements. Use fast libraries for locale-agnostic conversion:
   - value->string: https://github.com/fmtlib/fmt
   - string->double: https://github.com/lemire/fast_double_parser (10x
      faster double parsing according to their benchmark)

Bugfixes:
 - microsoft#2500
 - microsoft#2890
 - ninia/jep#205 (as it is related to LGBM as well)
    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    4bf22a9 View commit details
    Browse the repository at this point in the history

  2. Align CommonC namespace

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    538e102 View commit details
    Browse the repository at this point in the history

  3. Add new external_libs/ to python setup

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    bfcdb89 View commit details
    Browse the repository at this point in the history

  4. Try fast_double_parser fix #1 

    Testing commit e09e5aad828bcb16bea7ed0ed8322e019112fdbe

If it works it should fix more LGBM builds
    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    30ae0c0 View commit details
    Browse the repository at this point in the history

  5. CMake: Attempt to link fmt without explicit PUBLIC tag

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    3a97aba View commit details
    Browse the repository at this point in the history

  6. Exclude external_libs from linting

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    447050e View commit details
    Browse the repository at this point in the history

  7. Add exernal_libs to MANIFEST.in

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    37c0cd4 View commit details
    Browse the repository at this point in the history

  8. Set dynamic linking option for fmt.

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    6dcc5d0 View commit details
    Browse the repository at this point in the history

  9. linting issues

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    c7c1977 View commit details
    Browse the repository at this point in the history

  10. Try to fix lint includes

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    a206715 View commit details
    Browse the repository at this point in the history

  11. Try to pass fPIC with static fmt lib

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    f8556ab View commit details
    Browse the repository at this point in the history

  12. Try CMake P_I_C option with fmt library

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    18c6773 View commit details
    Browse the repository at this point in the history

  13. [R-package] Add CMake support for R and CRAN

    @jameslamb @AlbertoEAF
    jameslamb authored and AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    8d6ec54 View commit details
    Browse the repository at this point in the history

  14. Cleanup CMakeLists

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    859013a View commit details
    Browse the repository at this point in the history

  15. Try fmt hack to remove stdout

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    01a07a4 View commit details
    Browse the repository at this point in the history

  16. Switch to header-only mode

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    9ed5a39 View commit details
    Browse the repository at this point in the history

  17. Add PRIVATE argument to target_link_libraries

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    658b00b View commit details
    Browse the repository at this point in the history

  18. use fmt in header-only mode

    @jameslamb @AlbertoEAF
    jameslamb authored and AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    91dd2d8 View commit details
    Browse the repository at this point in the history

  19. Remove CMakeLists comment

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    f90fa3b View commit details
    Browse the repository at this point in the history

  20. Change OpenMP to PUBLIC linking in Mac

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    d062736 View commit details
    Browse the repository at this point in the history

  21. Update fmt submodule to 7.1.2

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    e3235fc View commit details
    Browse the repository at this point in the history

  22. Use fmt in header-only-mode

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    e4997bd View commit details
    Browse the repository at this point in the history

  23. Remove fmt from CMakeLists.txt

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    5c16913 View commit details
    Browse the repository at this point in the history

  24. Upgrade fast_double_parser to v0.2.0

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    03bd3b2 View commit details
    Browse the repository at this point in the history

  25. Revert "Add PRIVATE argument to target_link_libraries" 

    This reverts commit 3dd45dd.
    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    51843f7 View commit details
    Browse the repository at this point in the history

  26. Address James Lamb's comments

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    90f8bd8 View commit details
    Browse the repository at this point in the history

  27. Update R-package/.Rbuildignore 

    Co-authored-by: James Lamb <[email protected]>
    @AlbertoEAF @jameslamb
    AlbertoEAF and jameslamb committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    eda4759 View commit details
    Browse the repository at this point in the history

  28. Upgrade to fast_double_parser v0.3.0 - Solaris support

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    082c1d4 View commit details
    Browse the repository at this point in the history

  29. Use legacy code only in Solaris

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    cc19daf View commit details
    Browse the repository at this point in the history

  30. Fix lint issues

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    69d0488 View commit details
    Browse the repository at this point in the history

  31. Fix comment

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    1411133 View commit details
    Browse the repository at this point in the history

  32. Address StrikerRUS's comments (solaris ifdef).

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    ba28c0f View commit details
    Browse the repository at this point in the history

  33. Change header guards

    @AlbertoEAF
    AlbertoEAF committed Nov 24, 2020
    Configuration menu
    Copy the full SHA
    7e17729 View commit details
    Browse the repository at this point in the history