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

Use fmt C++ library? OPINIONS WANTED! #400

Open
mabruzzo opened this issue May 21, 2024 · 4 comments
Open

Use fmt C++ library? OPINIONS WANTED! #400

mabruzzo opened this issue May 21, 2024 · 4 comments
Labels
help wanted Extra attention is needed

Comments

@mabruzzo
Copy link
Contributor

This is an idea I had a few years back. I've gone back and forth on it a lot (I'm torn about whether the benefits outweigh the negatives... it seems a little close). I figured that I would throw it out there to see what people think

The problem

Using data to format strings in C++ is unsatisfactory. Historically, there are 2 approaches:

  1. use c-style printf-functions. (what we currently do)
  2. use C++-style streams like std::cout or std::cerr

Of these 2 options I'm generally a fan of the first since it is far less verbose and the data-formatting syntax is fairly "standard". However, the major drawback is that it can't properly handle any non-C primitives (e.g. it handles const char* strings, but not std::string or a user-defined custom string implementation).

The potential solution

The potential solution to is to make use of fmt C++ library (docs and GitHub repository). This is an extremely popular library that takes a printf-style approach to string formatting. In fact, it is so popular, that it has become part of the standard library (most features were added in C++20, a few more introduced in C++23 and C++26).

The basic premise is that it performs formatting based on python's str.format(*arg, **kwargs) formatting string. As a basic example, if one wrote

std::string s = fmt::format("This is a float {:.3f} and this is an int {}", 42.39542342, 42);

Then s would hold "This is a float 42.395 and this is an int 42".

Importantly, it provides:

  • support for formatting types in the standard library. This includes containers like std::vector. It also includes std::string (and supporting std::string is extremely important)1
  • a mechanism to allow formatting of custom types (which could help with debugging).
  • a fmt::format_to function that can be used to avoid heap allocations.

I think a major advantage is that in 3 to 6 years (whenever we update to future C++ releases), we could eventually remove fmt as a dependency and use the standard library instead (just like we did with boost::filesystem).

Drawbacks

The primary drawbacks is compilation times. I'm most concerned about this. Some compile-time and binary-size benchmarks were done (here). In that benchmark fmt takes 3 times longer to compile than printf and produces larger binaries. With that said, at runtime these other benchmarks suggest that fmt is generally ~20% faster than printf and an order of magnitude faster at formatting floats.

In theory, another drawback is that we're introducing a new dependency. However, I don't think this is a problem in practice.2

Summary

Again, I'm really torn on this. I think it all boils down to the question: "will we use this feature when we transition to C++20 and it becomes part of the standard library?"

  • If the answer is yes, then I think we should do it (since it will reduce the burden of transitioning new code added between now and then AND we get to enjoy the new feature now).
  • I tend to think that we want to use this feature -- I think it's hard to overstate the value in making it easy to write descriptive error messages (if it's difficult, then people are less likely to write them OR they may not make them descriptive).

I definitely want to hear people's opinions on this topic!

Footnotes

  1. Yes std::string has the .c_str() method which lets it work with normal printf. However, there is some "messiness" with making sure that the std::string is alive over the course of the printf-related function, that comes up when you call .c_str() on a temporary std::string that hasn't been stored in a variable (e.g. if foo() allocates and returns a string, then calling foo().c_str() may have some weird behavior). I believe that this "messiness" invokes undefined behavior (that may produce desired results on some platforms but not on others)

  2. Since fmt is small & lightweight and makes use of cmake (with modern practices), we could make Enzo-E's build have the default behavior of automatically downloading and installing it as a part of the build process. In other words, the end-user doesn't need to do anything. The only other dependency-related concern is compiler support (e.g. maybe a particular compiler has issues with it). Since this only uses standard C++ features this shouldn't be a problem. Plus I imagine that any compiler issues have been sorted out given the immense popularity of this library.

@mabruzzo mabruzzo added the help wanted Extra attention is needed label May 21, 2024
@jwise77
Copy link
Contributor

jwise77 commented May 22, 2024

This is new to me, so I don't have strong opinions on it. However, I use python f-strings, and I prefer its syntax over printf after years of using it. I also dislike the lengthy C++ stream syntax and have never used it heavily. I'd be willing to take the compilation time penalty to improve string formatting.

@bwoshea
Copy link
Contributor

bwoshea commented May 29, 2024

I also don't have a strong opinion, but I would tend to agree with John. The Python print formatting is very useful, and as the code gets increasingly broken down into subdirectories to make compilation faster I think that compiling as a potential issue will be less of a problem. Also, if this is the way that the language is going then we should adopt it sooner rather than later!

@gregbryan
Copy link
Contributor

Strong support for {fmt}! I have never been quiet about my dislike of iostreams.

@jobordner
Copy link
Contributor

I have no objection to people using fmt in principle. But keep in mind this is a parallel code: CkPrintf (or ckout) are preferred to printf (or cout), and Charm++ doesn't support fmt::print().

If you're guaranteed to be doing terminal I/O from the root chare, I think fmt::print() should be fine. I'm about 500x more concerned with runtime impacts of errant terminal outputs from 100K+ cores every cycle than any possible compile-time overhead, which I expect would be negligable.

I would recommend deferring widespread use of fmt::print() until Charm++ supports it, though fmt::format() coupled with CkPrintf, CkError, ckout, or ckerr should be fine.

Charm++ terminal I/O section

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants