Skip to content

[C++20] [Modules] Importing template-heavy library 3x slower than #include #61226

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.

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

Closed
JohelEGP opened this issue Mar 6, 2023 · 16 comments
Closed
Assignees
Labels
clang:modules C++20 modules and Clang Header Modules slow-compile

Comments

@JohelEGP
Copy link

JohelEGP commented Mar 6, 2023

At mpusz/mp-units#350 (comment) is a table that reveals that compiling the compile-time tests of a template-heavy library's module mp_units is 3x slower than using #includes.

Factoring out a lightweight module mp_units.core from mp_units, and changing the test unit_test to import that instead (and no other changes), reveals that simply importing mp_units results in terribly slow compilation. Below is a table comparing the compile times. This particular test was 5x slower than using #include, and 11x slower than using the lightweight module.

Compiled item #include mp_units.core mp_units
unit_test 2 s 1 s 11 s
@llvmbot
Copy link
Member

llvmbot commented Mar 6, 2023

@llvm/issue-subscribers-clang-modules

@koplas
Copy link
Contributor

koplas commented Mar 6, 2023

Can you compile with -ftime-trace and open the resulting json file with chrome://tracing in chrome?

I'm also experiencing increased build times with modules, so a flamegraph could give a hint if both issues are related.

@JohelEGP
Copy link
Author

JohelEGP commented Mar 6, 2023

This is how it looks.
1678131845

@koplas
Copy link
Contributor

koplas commented Mar 6, 2023

Could be related to #61040.

@JohelEGP JohelEGP changed the title [Modules] Importing template-heavy library 3x slower than #include [C++20] [Modules] Importing template-heavy library 3x slower than #include Mar 9, 2023
@ChuanqiXu9
Copy link
Member

@JohelEGP would you like to test again on the newest trunk?

@JohelEGP
Copy link
Author

Sure. I didn't know anything had changed.

@ChuanqiXu9
Copy link
Member

I fixed this one (#61064). But I am not sure if it can address your problem fully. If not, could you try to provide a reduced example? So that I can work on it quickly.

@JohelEGP
Copy link
Author

JohelEGP commented Mar 14, 2023

I see that you just closed the referenced #61040. I had been watching that one, since it was more relevant to me. I'll build and test tomorrow.

@ChuanqiXu9 ChuanqiXu9 self-assigned this Mar 14, 2023
@JohelEGP
Copy link
Author

Times are unchanged. I'll try to reduce it today.

@ChuanqiXu9
Copy link
Member

@JohelEGP how is it going? The issue report is a big concern for me.

@ChuanqiXu9
Copy link
Member

BTW, maybe you can try to remove all the constexpr/consteval things.

@JohelEGP
Copy link
Author

JohelEGP commented Mar 17, 2023

I tried for a day, and failed so far.

I merged the modules into the preprocessed mp_units.systems (equivalent to mp_units in the table) and unit_test (but takes me 3.4 s to compile rather than 11 s, because I defined some feature-stripping macros when compiling mp_units.systems), and appended their compile command line as a comment.

Remember that compiling unit_test with #include (without enabling modules, so there's no chance for include translation of standard C++ headers) takes 2 s (not feature-stripped). Compiling the preprocessed unit_test that imports mp_units.systems (feature-stripped) takes 3.4 s. I noticed that each type in unit_test adds a significant amount of overhead. Commenting out lines 144-179, so that there's only type metre, takes me 1.4 s to compile.

@JohelEGP
Copy link
Author

BTW, maybe you can try to remove all the constexpr/consteval things.

Definitely not all. I'd have to try that judiciously. After all, struct metre : named_unit<metre, "m"> {};, which passes a string literal as a template argument, needs to continue working. Compile-time is too ingrained into the library.

@ChuanqiXu9
Copy link
Member

BTW, maybe you can try to remove all the constexpr/consteval things.

Definitely not all. I'd have to try that judiciously. After all, struct metre : named_unit<metre, "m"> {};, which passes a string literal as a template argument, needs to continue working. Compile-time is too ingrained into the library.

I'm not suggesting you to do that. I just want to locate the problem.

@ChuanqiXu9
Copy link
Member

ChuanqiXu9 commented May 10, 2023

I image there will be some changes after b6c7177. @JohelEGP would you like to test it again?

@JohelEGP
Copy link
Author

Thank you! Seems fixed.

Updating the opening table:

Compiled item #include mp_units.core mp_units
unit_test before 2 s 1 s 11 s
unit_test after b6c7177 2 s 0 s 0 s

Updating the table from the issue linked in the OP:

Compiled item Time
Module mp_units before 33 s
Module mp_units after b6c7177 2 s
Static tests (#include) 1 m 11 s
Static tests (mp_units) before 3 m 34 s
Static tests (mp_units) after b6c7177 ???

As for the last row, now there are compilation errors. I'll have to investigate.

Do note that mp_units was made into one big module of all the would-be heavy submodules.
Even when all that mp_units did was reexport those submodules, it took about the same of time to compile as the modules it exported.
Now, there's still one such submodule, mp_units.systems, which still takes the 33 s to compile.
mp_units now takes 2 s, although all it does is reexport some modules and a few templates.
That's, perhaps, justified, as in the GMF it has: #include <gsl/gsl-lite.hpp>.

Module mp_units

module mp_units:

// The MIT License (MIT)
//
// Copyright (c) 2018 Mateusz Pusz
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in all
// copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
// SOFTWARE.

module;

#include <gsl/gsl-lite.hpp>

export module mp_units;

export import mp_units.core.fmt;
export import mp_units.core.io;
export import mp_units.systems;

#define UNITS_MODULE

export
{
#include <units/chrono.h>
}

<units/chrono.h>:

// The MIT License (MIT)
//
// Copyright (c) 2018 Mateusz Pusz
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in all
// copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
// SOFTWARE.

#pragma once

#ifndef UNITS_MODULE
#include <units/customization_points.h>
// IWYU pragma: begin_exports
#include <units/isq/si/time.h>
#include <units/point_origin.h>
#include <chrono>
// IWYU pragma: end_exports
#endif

namespace units {

template<typename Rep, typename Period>
struct quantity_like_traits<std::chrono::duration<Rep, Period>> {
private:
  static constexpr auto mag = ::units::mag<ratio(Period::num, Period::den)>();
public:
  using dimension = isq::si::dim_time;
  using unit = downcast_unit<dimension, mag>;
  using rep = Rep;
  [[nodiscard]] static constexpr rep number(const std::chrono::duration<Rep, Period>& q) { return q.count(); }
};

template<typename C>
struct clock_origin : point_origin<isq::si::dim_time> {};

template<typename C, typename Rep, typename Period>
struct quantity_point_like_traits<std::chrono::time_point<C, std::chrono::duration<Rep, Period>>> {
private:
  static constexpr auto mag = ::units::mag<ratio(Period::num, Period::den)>();
public:
  using origin = clock_origin<C>;
  using unit = downcast_unit<typename origin::dimension, mag>;
  using rep = Rep;
  [[nodiscard]] static constexpr auto relative(const std::chrono::time_point<C, std::chrono::duration<Rep, Period>>& qp)
  {
    return qp.time_since_epoch();
  }
};

namespace detail {

template<typename C, typename Rep, typename Period>
inline constexpr bool is_quantity_point_like<std::chrono::time_point<C, std::chrono::duration<Rep, Period>>> = true;

constexpr std::intmax_t pow_10(std::intmax_t v)
{
  gsl_Expects(v > 0);
  std::intmax_t res = 1;
  for (std::intmax_t i = 0; i < v; i++) res *= 10;
  return res;
}

template<ratio R>
constexpr auto to_std_ratio_impl()
{
  return std::ratio<R.num, R.den>{};
}

}  // namespace detail

// TODO ICE below on gcc-11 when `ratio` is used instead of `auto`
template<auto R>
using to_std_ratio = decltype(detail::to_std_ratio_impl<R>());

template<typename U, typename Rep>
[[nodiscard]] constexpr auto to_std_duration(const quantity<isq::si::dim_time, U, Rep>& q)
{
  return std::chrono::duration<Rep, to_std_ratio<as_ratio(U::mag)>>(q.number());
}

template<typename C, typename U, typename Rep>
[[nodiscard]] constexpr auto to_std_time_point(const quantity_point<clock_origin<C>, U, Rep>& qp)
{
  using ret_type = std::chrono::time_point<C, std::chrono::duration<Rep, to_std_ratio<as_ratio(U::mag)>>>;
  return ret_type(to_std_duration(qp.relative()));
}

}  // namespace units

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:modules C++20 modules and Clang Header Modules slow-compile
Projects
None yet
Development

No branches or pull requests

5 participants