[RFC] Removing Waymarking from Use.

Project Infrastructure LLVM Dev List Archives
#1

Hi llvm-dev,

I have a patch open for review that removes waymarking https://reviews.llvm.org/D77144.

This patch removes waymarking and replaces it with storing a pointer to the User in the Use.
when compiling the CTMark tests of the test suite, this give an average of +1.8% max memory use and -1.1% compile time.

Removing Waymarking also simplifies the code of Use and User.

Any thought?

Cheers,

Gauthier

#2

Waymarking is very complicated, and I’d be generally +1 on removing it if it is not paying for itself. It is also nice to see compiler speedups!

I’m curious about kimwitu++ though - when waymarking initially landed, it was a significant memory size win for that. Do you have any idea why you’re not seeing the same difference? Is it possible that different things were being measured back then?

-Chris

#3

I’m in favor for this change.

I’ve also done some testing myself and noticed only about 1-2% increase of memory, in exchange for about 1-3% increase of speed.
I can’t say that a speedup of 3% (at most, and usually 1%), worth working for, this is a simple change, that give a lot more than this; especially simpler code path (also easier debugging).

As part of my measurements, while compiling “deal.II”, the maximum number of simultaneous Use instances was 2,728,963 (and in most cases, about half of it).
In a 64-bit process (Clang for that matter), we will have roughly 21 MB of memory spent on the extra pointer (of 8 bytes size).
On a 16-core machine, for which, in many cases, a build will trigger that many simultaneous Clang processes, we will reach, in the worst case, 336 MB more memory spent on compilation.

Is it worth it? I think it is. But I am not sure I see the whole picture - are there low-memory systems that need to run LLVM on?

I am not sure what needs to be done to approve such a fundamental change; especially when we can’t prove the Waymarking was needed at all.

#4

I'm in favor for this change.

I've also done some testing myself and noticed only about 1-2% increase of
memory, in exchange for about 1-3% increase of speed.
I can't say that a speedup of 3% (at most, and usually 1%), worth working
for, this is a simple change, that give a lot more than this; especially
simpler code path (also easier debugging).

As part of my measurements, while compiling "deal.II", the maximum number
of simultaneous `Use` instances was 2,728,963 (and in most cases, about
half of it).
In a 64-bit process (Clang for that matter), we will have roughly 21 MB of
memory spent on the extra pointer (of 8 bytes size).
On a 16-core machine, for which, in many cases, a build will trigger that
many simultaneous Clang processes, we will reach, in the worst case, 336 MB
more memory spent on compilation.

Is it worth it? I think it is. But I am not sure I see the whole picture -
are there low-memory systems that need to run LLVM on?

I am not sure what needs to be done to approve such a fundamental change;
especially when we can't prove the Waymarking was needed at all.

I guess if no-one brings forth arguments (= results) for keeping it and

people continue to support replacing it, we will replace it. There should

be a grace period in which people have the chance to do their benchmarking

(basically what is happening), but I don't recall a problem being reported yet.

Cheers,

Johannes

P.S. Thanks for sharing your findings!

#5

I agree. I’m not hearing strong arguments to retain it, so let’s remove it. Worst case, we can always reinstate it if there is a good reason discovered down the line. Thank you!

-Chris

#6

a bit of time has passed and there to my knowledge still no strong arguments against removing it.
is everyone OK to proceed with the removal ?

Gauthier

#7

Yes please.

#8

Maybe we can utilize the implementation in mlir/IR/UseDefLists.h in here (clearly it is superior to llvm::Use) ?
By that we will have the same code base (instead of duplicate implementations of Use-Lists).

D77144 should definitely go in, first, though.

#9

Now that D77144 has landed; any thoughts regarding what I suggested? Using UseDefLists.h?

#10

Maybe you could prepare a RFC explaining pros and cons?

#11

From looking at UseDefLists.h, it seems generic enough to reimplement Uses in llvm with it.

but I don’t think we are gaining much by doing it. Why do you think it is “clearly it is superior to llvm::Use” ?