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.

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

Q2 2021 priorities #1800

Closed
StephanTLavavej opened this issue Apr 2, 2021 · 16 comments
Closed

Q2 2021 priorities #1800

StephanTLavavej opened this issue Apr 2, 2021 · 16 comments
Labels
announcement Announcements related to the repo

Comments

@StephanTLavavej
Copy link
Member

In the second quarter of 2021, our top priority (again) is finishing C++20 for VS 2019 16.10. (See our C++20 Features project and Conformance milestone.) We continue to welcome PRs that fix bugs, improve performance/throughput, and so forth, but code reviews may be significantly delayed as we focus on C++20. We expect to be done in mid-April.

After finishing C++20, we'll have the capacity to work on:

  • Reviewing the PR backlog
  • Completing the GitHub migration
  • Open-sourcing vcruntime/vcstartup as part of this repo

At this time, we aren't planning to start work on the vNext ABI-incompatible release in this quarter. vNext is different from "Dev17" 17.0, which will be ABI-compatible. Except for PRs specifically ported to 16.10, all changes are automatically flowing into 17.0 now, as recorded in the Changelog.

If you have questions about our plans for this quarter, feel free to ask them here. (Note that compiler, IDE, etc. priorities are off-topic.)

@StephanTLavavej StephanTLavavej added the announcement Announcements related to the repo label Apr 2, 2021
@StephanTLavavej StephanTLavavej pinned this issue Apr 2, 2021
@jovibor
Copy link
Contributor

jovibor commented Apr 2, 2021

our top priority (again) is finishing C++20 for VS 2019 16.10

Does it mean that after 16.10 is released the C++20 STL ABI will be frozen for, at the very least, until vNext is out?
Or are you planning to fix/tweak new C++20 features for some time after release (in 16.11, 16.12, 17.0...)?

@fsb4000
Copy link
Contributor

fsb4000 commented Apr 2, 2021

finishing C++20 for VS 2019 16.10

So will "format" be ported to VS 2019? That would be great, I thought "format" and "chrono" will be landed in Dev17(VS 2021?)...

@StephanTLavavej
Copy link
Member Author

@jovibor

Does it mean that after 16.10 is released the C++20 STL ABI will be frozen for, at the very least, until vNext is out?

That's correct, C++20 will join C++14/17 in being ABI-frozen until vNext. (This coincides with the addition of the /std:c++20 compiler option.)

We'll still be able to fix things in ways that preserve ABI, but we won't be able to change layout (as we did with span), remove exports, etc.


@fsb4000

So will "format" be ported to VS 2019?

Yep! We're trying to finish it in time to be ported to VS 2019 16.10 Preview 3. (There will also be a VS 2019 16.11 but we really don't want the features to slip into that release.)

@jovibor
Copy link
Contributor

jovibor commented Apr 2, 2021

That's correct, C++20 will join C++14/17 in being ABI-frozen until vNext.

Thank you.
Well, that's expected and a bit unfortunate.

For example, the fmt library, which aforementioned std::format is based on, is constantly evolving, gaining additional speed improvements, fixing bugs, etc...
But std::format in STL once released will be stuck in time for years I guess.
So please, before releasing make sure it's at least comparable in terms of speed with fmt, so we won't have a situation as is with std::regex which is very slow :)

@sylveon
Copy link
Contributor

sylveon commented Apr 2, 2021

Also I'm afraid P2216 might cause complications since it involves ABI changes and WG21 seems to want to retroactively apply it to C++20.

@StephanTLavavej
Copy link
Member Author

@jovibor We'll have a brief window (basically 16.10 Preview 4) where we can make ABI-breaking performance improvements. If you want to help, you can investigate feature/format's performance right now and identify any deficiencies. We're putting all of our effort into just finishing the feature so while we're keeping an eye on performance, we won't have time for detailed performance work until it's complete.

The main thing to look at first would be any persistent data structures (those that live longer than a function call) because their representations will be frozen. We can arbitrarily modify functions (if they're header-only), including data structures that remain entirely within those functions, but anything that can be mixed-and-matched across TUs is subject to ABI considerations (which is why regex is such a headache, but charconv is not in that way).

I would love to have additional time for the ABI to be "unlocked" (and I want a 🐴 and a 🦄), but this decision is from our bosses and boss-like entities - when we add /std:c++20 and tell customers that it's ready for production use, it needs to follow the same ABI rules as everything else, because otherwise it would be confusing and a source of pain.


@sylveon

Also I'm afraid P2216 might cause complications since it involves ABI changes and WG21 seems to want to retroactively apply it to C++20.

Yeah, we're aware of this. We don't want to implementer-veto anything but we will if we have no other choice. (An implementer veto is when the Standard tells us to do something that's impossible, so we just don't do it - as happened with regex::multiline). WG21 can't pretend that Standard Library implementation schedules are unknowable - we're all open-source now!

@jovibor
Copy link
Contributor

jovibor commented Apr 3, 2021

We can arbitrarily modify functions (if they're header-only), including data structures that remain entirely within those functions, but anything that can be mixed-and-matched across TUs is subject to ABI considerations

But isn't the std::format a header-only feature?
https://github.com/microsoft/STL/blob/feature/format/stl/inc/format

At least I can compile it (by copy-paste) and it seems to work as expected.
Or I'm missing something?

@StephanTLavavej
Copy link
Member Author

It's header-only, but there's a difference between this:

inline int compute_something(int x, int y) {
    // Arbitrarily complicated computation involving x and y.
    // This implementation can be changed without breaking ABI.
}

And this:

struct Span {
    int * ptr;
    size_t len;
    // This is also header-only, but reordering ptr and len would break ABI.
};

To determine whether a change could break ABI, one has to reason about what happens when an old object file (built with the old implementation) and a new object file (built with the new implementation) are mixed together. It's okay for them to have different implementations of compute_something() because the linker will pick one (and they have the same interface), but it's not okay for them to disagree on the offset of Span::ptr or Span's total size, etc.

@jovibor
Copy link
Contributor

jovibor commented Apr 3, 2021

To determine whether a change could break ABI, one has to reason about what happens when an old object file

That's understood. I just didn't know that STL is so careful about object files compatibility, my naive assumption was that this is only for .dll boundaries.

If you want to help, you can investigate feature/format's performance right now and identify any deficiencies.

So, below are some of my rough test results.
Everything was compiled under MSVS 16.9.3, Release/x64 /O2:

#include <iostream>
#include <chrono>
#include "format" //copy-pasted from https://github.com/microsoft/STL/blob/feature/format/stl/inc/format

#define FMT_HEADER_ONLY
#include "fmt/format.h" //https://github.com/fmtlib/fmt copy of the current master branch.

int main()
{
	const auto tSTDStart = std::chrono::high_resolution_clock::now();
	for (auto i = 0; i < 1'000'000; ++i) //STD test loop.
		const auto wstrSTD = std::format(L"std::format {}\n", 42);
	const auto tSTDEnd = std::chrono::high_resolution_clock::now();

	const auto tFMTStart = std::chrono::high_resolution_clock::now();
	for (auto i = 0; i < 1'000'000; ++i) //FMT test loop.
		const auto wstrFMT = fmt::format(L"fmt::format {}\n", 42);
	const auto tFMTEnd = std::chrono::high_resolution_clock::now();
	
	const auto durationSTD = std::chrono::duration_cast<std::chrono::microseconds>(tSTDEnd - tSTDStart).count();
	const auto durationFMT = std::chrono::duration_cast<std::chrono::microseconds>(tFMTEnd - tFMTStart).count();

	const auto wstrResult = std::format(L"STD duration: {}\nFMT duration: {}\nRatio is (std/fmt): {:.2f}\n",
		durationSTD, durationFMT, static_cast<double>(durationSTD) / static_cast<double>(durationFMT));

	std::wcout << wstrResult << std::endl;

	return 0;
}

The output is:

STD duration: 227888
FMT duration: 184730
Ratio is (std/fmt): 1.23

I ran this simple program many times and the ratio was almost always somewhere in-between 1.20-1.25 (rare times 1.3).
So, at the moment std::format implementation is about 20-25% slower than fmt, at least for such simple cases.

@jovibor
Copy link
Contributor

jovibor commented Apr 3, 2021

If the test is just a bit more complicated then the ratio is even worse for std::format:

#include <iostream>
#include <chrono>
#include "format" //copy-pasted from https://github.com/microsoft/STL/blob/feature/format/stl/inc/format

#define FMT_HEADER_ONLY
#include "fmt/format.h" //https://github.com/fmtlib/fmt copy of the current master branch.

int main()
{
	const auto tSTDStart = std::chrono::high_resolution_clock::now();
	size_t sum1 { };
	for (auto i = 0; i < 1'000'000; ++i) //STD test loop.
	{
		const auto wstrSTD = std::format(L"std::format {}\n", i);
		sum1 += wstrSTD.size();
	}
	const auto tSTDEnd = std::chrono::high_resolution_clock::now();

	const auto tFMTStart = std::chrono::high_resolution_clock::now();
	size_t sum2 { };
	for (auto i = 0; i < 1'000'000; ++i) //FMT test loop.
	{
		const auto wstrFMT = fmt::format(L"fmt::format {}\n", i);
		sum2 += wstrFMT.size();
	}
	const auto tFMTEnd = std::chrono::high_resolution_clock::now();

	const auto durationSTD = std::chrono::duration_cast<std::chrono::microseconds>(tSTDEnd - tSTDStart).count();
	const auto durationFMT = std::chrono::duration_cast<std::chrono::microseconds>(tFMTEnd - tFMTStart).count();

	const auto wstrResult = std::format(L"STD duration: {}\nFMT duration: {}\nRatio is (std/fmt): {:.2f}\n",
		durationSTD, durationFMT, static_cast<double>(durationSTD) / static_cast<double>(durationFMT));

	std::wcout << wstrResult << std::endl;

	return int(sum2 - sum1);
}
STD duration: 328497
FMT duration: 193228
Ratio is (std/fmt): 1.70

@StephanTLavavej
Copy link
Member Author

I just didn't know that STL is so careful about object files compatibility, my naive assumption was that this is only for .dll boundaries.

Yeah, it's because people distribute both static libraries and DLLs.

I've filed #1802 to investigate improving <format>'s performance in the future - thanks for the test cases. (Due to the timeline - literally two weeks remaining - we are extremely limited in what we can do in the immediate future.)

@jovibor
Copy link
Contributor

jovibor commented Apr 4, 2021

Thanks for the feedback.
Interestingly enough, when testing format_to counterparts (with preallocated out-buffer), the difference is way less significant.

std::format_to duration: 123268
fmt::format_to duration: 118632
Ratio is (std/fmt): 1.04

It varies from 1.02 to 1.06.
So maybe it's somehow related to not very optimal std::wstring handling/preallocating in the std::format case (just an assumption).

@MikeGitb
Copy link

MikeGitb commented Apr 5, 2021

It's okay for them to have different implementations of compute_something() because the linker will pick one (and they have the same interface), but it's not okay for them to disagree on the offset of Span::ptr or Span's total size, etc.

Not sure if and when that is a problem in practice, but isn't it also dangerous if pre and/or post conditions of a function changes, even if the signature remains the same?

@StephanTLavavej
Copy link
Member Author

Yes, that can be dangerous - it's typically a concern for internal functions, when we make coordinated changes to two or more functions. (Thus we have a few "v2", "v3", etc. functions/classes, which avoids any old TUs accidentally picking up new implementations partially.) Interestingly, if the signature changes and we're using C++ mangling, it's fine - the different mangled name is like a rename. It's not typically a concern for Standard functions where the pre/post-conditions aren't under our control.

An entire book could be written about making ABI-safe changes.

@PiliLatiesa
Copy link

We don't want to implementer-veto anything but we will if we have no other choice.

Sad to hear. If if not mistaken P2216 is already scheduled for plenary vote.

@brevzin
Copy link

brevzin commented Apr 8, 2021

[STL]

@jovibor

Does it mean that after 16.10 is released the C++20 STL ABI will be frozen for, at the very least, until vNext is out?

That's correct, C++20 will join C++14/17 in being ABI-frozen until vNext. (This coincides with the addition of the /std:c++20 compiler option.)

We'll still be able to fix things in ways that preserve ABI, but we won't be able to change layout (as we did with span), remove exports, etc.

[STL]

Yeah, we're aware of this. We don't want to implementer-veto anything but we will if we have no other choice. (An implementer veto is when the Standard tells us to do something that's impossible, so we just don't do it - as happened with regex::multiline). WG21 can't pretend that Standard Library implementation schedules are unknowable - we're all open-source now!

What does this mean in light of multiple in-flight Ranges papers which have ABI impact (split, default construction, and join)? As well as the still-being-designed user-facing design for range adaptor closure objects so that users can write their own range adaptors? @CaseyCarter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
announcement Announcements related to the repo
Projects
None yet
Development

No branches or pull requests

7 participants