Hacker News Reader

Comment by SleepyMyroslav | Hacker News Reader

Two notes from C++ user in gamedev. Please, make realistic ValWithPtr implementation for C++. Please, implement comparison inlining in C and C++ where algorithm API allows it.

View on HN

The C and C++ code pulls in all thirdparty code as header only, making for one single translation unit, and the compiler is able to do comparison inlining for the C++ implementations. I've written a bit about this previously [here](https://github.com/Voultapher/sort-research-rs/blob/main/wri...).

Regarding `ValWithPtr` my goal was to make it as close semantically as I could to the Rust code while keeping the example to a minimum to avoid distracting from the main point. If you have a concrete idea how `ValWithPtr` could have been modeled better given these criteria, please let me know.

I’ll allow that I’m entirely misunderstanding the goal, but why is it important to stick close to the semantic meaning of rust code?

How does this differ from implementing GC in C++ then showing how much better Java does?

I'll try to explain it. I want to demonstrate that something can be used incorrectly and cause issues. To reach a wider audience and make the claims comparable, I wanted to show the same logic in C++ and Rust. For ValWithPtr there was pretty much only one way to do it in Rust, so I used that as the baseline and tried to mirror the semantics and layout of that type. Does that make sense?

Not really. It comes across as an extremely contrived example, motivated by a desire to show one language is better, and misusing the other language in order to arrive at that goal. In other words, typical tiresome language evangelism. I think your abuse of "mutable" is about on the same level of someone writing bogus code in an "unsafe" block and then complaining that his foot is gone and Rust did nothing to prevent it.

No one sane would mutate elements, free memory or throw exceptions in a sort comparison function. To demonstrate fairness, you therefore need to at least also show how you could use Rust "unsafe" to break the sort, which is also a language feature you probably wouldn't want to use in this context.

> No one sane would mutate elements, free memory or throw exceptions in a sort comparison function.

No one sane would intentionally do so, but we are not talking about that. The OP has a section that links to a blog post by Danila Kutenin, which mentions `std::sort` often just segfaults if a comparator doesn't satisfy irreflexivity or asymmetry, yet such comparators were found in the wild, even passing all reviews and even tests (!) because this behavior greatly depends on the number of elements. Given this occurrence of the actual bug in user comparators, it should be no surprise that they may also contain mutations, memory deallocations and exception throws (all of which can easily be a side effect from internal routines).

It is a surprise. I have seen and written buggy comparators that violated strict weak ordering (these usually involve floating points...). But I have never seen mutating or throwing comparators.

It is great that rust can prevent them those additional issues, but in the grand scheme of things it is not something I lose sleep about.

TBH I find it difficult to understand:

1. How do you compile the C++ code? E.g. what flags do you use with GCC, clang and MSVC

2. How exactly is C++ binary run through a Rust benchmark suite? FFI?

3. Is it possible for rust code-under-test to be at any advantage here because it is run and built natively from the benchmark written in the rust itself?

What I find questionable though is the actual C++ code found in the benchmark:

1. Exception-handling code around std::sort and std::stable_sort - nobody does that. What problem are you trying to solve with this? Comparators do not throw exceptions.

2. Using function pointers for the comparators - surprising to see such code in C++ benchmark - it's very non-idiomatic and essentially making it impossible for a compiler to inline the code. std::sort is rarely used like that.

3. Passing over some magical third argument to the comparator function - ?

4. Passing over the context to the comparator "function" - I fail to see if "ctx" has been used anywhere?

5. "Making" the comparator function with make_compare_fn which in turn instantiates a lambda that, again, throws an exception from its body.

6. Storing a comparison result - why? You only need to return true or false from a comparator.

7. Modeling rust "panics" by storing a boolean is_panic plus exception-handling code plus throwing exceptions when is_panic is "ON" - why?

8. Confusing exceptions with rust panics - even if you wanted to do so, which still would be an arguable thing to do, std::abort or std::terminate is a replacement for std::panic. Not throwing exceptions and implementing the is_panic logic around it.

9. What is https://github.com/Voultapher/sort-research-rs/blob/main/src... being used for?

I think you're making a bit dishonest representation here for the reasons above. And I have not delved very much into depth nor have I covered all the code but just the fragment of it. Also, it is not quite clear how everything is put together and run because, ideally, you would want to have multiple binaries built with their representative toolchains/scripts _regardless_ of your benchmarking framework. And only then I would want to point the benchmarking framework to the respective binary to run the test. Here, it seems it's the other way around.

You claim yourself that "I have not delved very much into depth" yet you make a lot of assumptions here. Here are the key facts, the `_by` functions that contain the complex exception handling stuff are only used for tests and the benchmark ones are as native as possible e.g. `std::sort(data, data + len)`. I pick either gcc or clang based on what gives better performance, and use these settings with the cc crate https://github.com/Voultapher/sort-research-rs/blob/b131ed61... plus sort specific options. It all gets statically linked into one big binary. Also note the C++ code is header only, so one big translation unit per sort. The only disadvantage based on me looking at the generated asm is one additional function call versus possible inlining for the initial part of the sort. This adds a total of ~20 or less cycles to the total time, which in the benchmark is measured in microseconds, where 1 microsecond is ~4900 cycles.

And yet you ignore majority of my questions above. It was an honest feedback but now I will bite and say that the report looks "good on the paper" but the underlying code is low-quality, very non-idiomatic, without explanations, very difficult to understand on what and how it is being tested, testing/benchmarking methodology etc. Many important pieces are missing and all that poses a question on what genuine intention of this experiment was - was it to actually find a bottleneck in implementations, if any, or was it something else. Now, that you're avoiding to address the feedback, it begins to become obvious.

C and C++ are often used to achieve max performance. For max performance devs have to guarantee inlining. Which means programmers intentionally choose bigger code size and instantiation of sort algorithm code for every type and comparison combination. Function pointers to comparison operators are not relevant for max performance part of the community.

For ValWithPtr to make sense it needs its semantics defined. Correct C++ types can be deep copying, move only, reference counted, singleton instance etc. For example typed deep copy version without attempting any verification (link to compiler explorer): https://godbolt.org/z/3sesYY8of

If it is not helpful for you please feel free to ignore all of the above.

I'm sure the author would accept pull requests if you've got the time to show ValWithPtr safety.

Reply on news.ycombinator.com