opencog / cogutil

Very low-level C++ programming utilities used by several components
Other
39 stars 78 forks source link

Upgrade to C++17? #177

Closed ngeiswei closed 4 years ago

ngeiswei commented 5 years ago

Is it OK if we move to the C++17 standard?

That way no need to rely on boost for multi-threading programming goodies like shared_mutex. I must warn you though that it would likely require to increase gcc minimum version to 6, as according to https://en.cppreference.com/w/cpp/compiler_support#cpp17 gcc 5 has poor support for C++17.

linas commented 5 years ago

On my devel system, I'm running debian stable, which uses gcc version 6.3.0 On my data-processing LXC containers, I'm using ubuntu 16.04 which has gcc-5.4.0 and would rather not disrupt those for the next few weeks or a bit more. It took me the last month to get everything stable and they're finally running and producing results, so I'm nervous about downtime. But maybe in a week or two...

linas commented 5 years ago

Also should poll the HK labs.

ngeiswei commented 5 years ago

OK, meanwhile I'll use boost, since C++17 provides pretty much a drop-in replacement for it, shouldn't be too much of a head-heck later on.

linas commented 5 years ago

Do you want to use these mutexes for atomspace, or for other repos (moses/as-moses/opencog)? I only need the atomspace in these old containers; I could simply avoid pulling a new cogutils. I want to retain the ability to pull the newest atomspace, because I keep tripping over bugs in it. I have no other dependencies...

ngeiswei commented 5 years ago

Those mutexes are for the URE. MOSES uses boost mutexes and they seem rather untroublesome.

linas commented 5 years ago

OK. Well, anyway, double-check your thinking to make sure you really need these. Other code in the atomspace seems to get by with just a plain mutex, or a recursive mutex. You might be setting yourself up for trouble by using shared mutex. Think about how to simplify the design, so that you don't need one.

Also, recall, FYI, that cogutils has thread-safe queues(fifo) and stacks(lifo) that are nearly identical to the std:: versions thereof.

ngeiswei commented 5 years ago

Yes, I might use plain mutex for now, it's just that I've learned while working on multithreaded MOSES that shared mutex on read can improve performance, though I forgot the numbers and foolishly didn't document that is a diary.

linas commented 5 years ago

Yeah, I recall struggling with a desire for a fast read mutex, but I forget what. I do vaguely recall discovering that read-write mutexes were really slow. (about 4-5 years ago!?) For simple reads, there are always atomics :-) And then there are semaphores (the queue/stack uses semaphores so that multiple readers all block on the queue, waiting for some drive-by shooter to put something into the queue.)

linas commented 4 years ago

I'm free and clear now, upgrade to gcc 6 as desired.

ngeiswei commented 4 years ago

OK, so it seems @linas and @vsbogd would be OK with a C++17 upgrade. Anybody else?

ngeiswei commented 4 years ago

BTW, C++17 has some nice additions such as

https://en.cppreference.com/w/cpp/language/structured_binding

linas commented 4 years ago

double-check with @radivarig who is busy maintaining nix packages.

My main devel system (debian) seems to be gcc-8.3 now. My ubuntu 18.04 containers have gcc-7.4 on them.

Radivarig commented 4 years ago

@linas Sure! I can build with gcc 8.3

-- The C compiler identification is GNU 8.3.0
-- The CXX compiler identification is GNU 8.3.0

Currently it's 7.4, but any of these are available: 4.8.5, 4.9.4, 5.5.0, 6.5.0, 7.4.0, 8.3.0, 9.1.0.

vsbogd commented 4 years ago

JIC my dev machine is Ubuntu 18.04 LTS and it has gcc 7.4 installed, so I would not make version bigger than 7.4.

linas commented 4 years ago

FYI, I have a pattern-matcher benchmark, opencog/benchmark/query-link which I measured in July (5 months ago) and today. I compared perf on the same git commit, so the only difference should be due to different compilers and/or glibc. And there's a HUGE difference! Like 40% -- a drop from 125 second to 89 seconds. I failed to write down the compiler for the july run, but In think it was gcc-6.3.0 and glic-2.24 vs. today its gcc-8.3.0 and glibc-2.28

I mention glibc not just the compiler, because a lot of cpu-time is spend in std::__shared_ptr<opencog::Atom., etc. and in std::_Rb_tree<opencog::Handle, etc. which are part of glibc not gcc.

linas commented 4 years ago

Retested on ubuntu 16.04 LTS gcc version 5.4.0 and glibc-2.23 and indeed, the new compiler is a lot faster.. for the pattern matcher only. The other benchmarks seem to be unaffected.