[PATCH] D57986: [ProfileData] Remove non-determinism: Change DenseMap to MapVector

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D57986#1398271 , @vsk wrote:

> I think this could roughly double the memory utilization of the writer, which 
> might be problematic because the number of records to write can be high. 
> (@dblaikie did some work on reducing memory usage in this area, he might have 
> hard data about this.)
>
> As the write should only occur once, maybe the better tradeoff would be to 
> sort?


Yeah, unless I'm missing something (quite possibly) this seems like the sort of 
place where "do a bunch of processing, then sort, then iterate" is a valid 
strategy.

@mgrang - does it look like any use case is emitting more than once & modifying 
in between? I would find that pretty surprising. I think it's just "build build 
build, stop building, emit" and adding a sort-before-emit would be OK there.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57986/new/

https://reviews.llvm.org/D57986



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57986: [ProfileData] Remove non-determinism: Change DenseMap to MapVector

[Vedant Kumar via Phabricator via cfe-commits]

vsk added subscribers: dblaikie, vsk.
vsk added a comment.

I think this could roughly double the memory utilization of the writer, which 
might be problematic because the number of records to write can be high. 
(@dblaikie did some work on reducing memory usage in this area, he might have 
hard data about this.)

As the write should only occur once, maybe the better tradeoff would be to sort?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57986/new/

https://reviews.llvm.org/D57986



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57986: [ProfileData] Remove non-determinism: Change DenseMap to MapVector

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

Ping for reviews please.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57986/new/

https://reviews.llvm.org/D57986



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57986: [ProfileData] Remove non-determinism: Change DenseMap to MapVector

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

tools/llvm-profdata/instr-remap.test is failing in the reverse iteration bot. 
See 
http://lab.llvm.org:8011/builders/reverse-iteration/builds/10546/steps/check_all/logs/stdio.

This is because FunctionData is iterated in InstrProfWriter to output function 
counts, etc. But for two functions with the same name the iteration order is 
not defined if we use a DenseMap. Changing this to MapVector resolves this.

Note: We can also sort FunctionData before iteration to fix this issue. But 
that would mean we would need to sort every time we iterate in order to print. 
Using a MapVector instead is a better option IMO.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57986/new/

https://reviews.llvm.org/D57986



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57986: [ProfileData] Remove non-determinism: Change DenseMap to MapVector

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang created this revision.
mgrang added reviewers: rsmith, bogner.
Herald added a project: clang.

Repository:
  rC Clang

https://reviews.llvm.org/D57986

Files:
  include/llvm/ProfileData/InstrProfWriter.h


Index: include/llvm/ProfileData/InstrProfWriter.h
===
--- include/llvm/ProfileData/InstrProfWriter.h
+++ include/llvm/ProfileData/InstrProfWriter.h
@@ -14,7 +14,7 @@
 #ifndef LLVM_PROFILEDATA_INSTRPROFWRITER_H
 #define LLVM_PROFILEDATA_INSTRPROFWRITER_H
 
-#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Support/Endian.h"
@@ -32,7 +32,7 @@
 
 class InstrProfWriter {
 public:
-  using ProfilingData = SmallDenseMap;
+  using ProfilingData = MapVector;
   enum ProfKind { PF_Unknown = 0, PF_FE, PF_IRLevel };
 
 private:


Index: include/llvm/ProfileData/InstrProfWriter.h
===
--- include/llvm/ProfileData/InstrProfWriter.h
+++ include/llvm/ProfileData/InstrProfWriter.h
@@ -14,7 +14,7 @@
 #ifndef LLVM_PROFILEDATA_INSTRPROFWRITER_H
 #define LLVM_PROFILEDATA_INSTRPROFWRITER_H
 
-#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Support/Endian.h"
@@ -32,7 +32,7 @@
 
 class InstrProfWriter {
 public:
-  using ProfilingData = SmallDenseMap;
+  using ProfilingData = MapVector;
   enum ProfKind { PF_Unknown = 0, PF_FE, PF_IRLevel };
 
 private:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits