[PATCH] D58584: [XRay][tools] Revert "Use Support/JSON.h in llvm-xray convert"

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 23 14:21:16 PST 2019


lebedev.ri created this revision.
lebedev.ri added reviewers: dberris, kpw, sammccall.
lebedev.ri added a project: LLVM.
Herald added subscribers: jdoerfert, courbet.

This reverts D50129 <https://reviews.llvm.org/D50129> / rL338834 <https://reviews.llvm.org/rL338834>: [XRay][tools] Use Support/JSON.h in llvm-xray convert

Abstractions are great.
Readable code is great.
JSON support library is a *good* idea.

However unfortunately, there is an internal detail that one needs
to be aware of in `llvm::json::Object` - it uses `llvm::DenseMap`.
So for **every** `llvm::json::Object`, even if you only store a single `int`
entry there, you pay the whole price of `llvm::DenseMap`.

Unfortunately, it matters for `llvm-xray`.

I was trying to analyse the `llvm-exegesis` analysis mode performance,
and for that i wanted to view the LLVM X-Ray log visualization in Chrome
trace viewer. And the `llvm-xray convert` is sluggish, and sometimes
even ended up being killed by OOM.

`xray-log.llvm-exegesis.lwZ0sT` was acquired from `llvm-exegesis`
(compiled with ` -fxray-instruction-threshold=128`)
analysis mode over `-benchmarks-file` with 10099 points (one full
latency measurement set), with normal runtime of 0.387s.

Timings:
Old: (copied from D58580 <https://reviews.llvm.org/D58580>)

  $ perf stat -r 5 ./bin/llvm-xray convert -sort -symbolize -instr_map=./bin/llvm-exegesis -output-format=trace_event -output=/tmp/trace.yml xray-log.llvm-exegesis.lwZ0sT 
  
   Performance counter stats for './bin/llvm-xray convert -sort -symbolize -instr_map=./bin/llvm-exegesis -output-format=trace_event -output=/tmp/trace.yml xray-log.llvm-exegesis.lwZ0sT' (5 runs):
  
            21346.24 msec task-clock                #    1.000 CPUs utilized            ( +-  0.28% )
                 314      context-switches          #   14.701 M/sec                    ( +- 59.13% )
                   1      cpu-migrations            #    0.037 M/sec                    ( +-100.00% )
             2181354      page-faults               # 102191.251 M/sec                  ( +-  0.02% )
         85477442102      cycles                    # 4004415.019 GHz                   ( +-  0.28% )  (83.33%)
         14526427066      stalled-cycles-frontend   #   16.99% frontend cycles idle     ( +-  0.70% )  (83.33%)
         32371533721      stalled-cycles-backend    #   37.87% backend cycles idle      ( +-  0.27% )  (33.34%)
         67896890228      instructions              #    0.79  insn per cycle         
                                                    #    0.48  stalled cycles per insn  ( +-  0.03% )  (50.00%)
         14592654840      branches                  # 683631198.653 M/sec               ( +-  0.02% )  (66.67%)
           212207534      branch-misses             #    1.45% of all branches          ( +-  0.94% )  (83.34%)
  
             21.3502 +- 0.0585 seconds time elapsed  ( +-  0.27% )

New:

  $ perf stat -r 9 ./bin/llvm-xray convert -sort -symbolize -instr_map=./bin/llvm-exegesis -output-format=trace_event -output=/tmp/trace.yml xray-log.llvm-exegesis.lwZ0sT
  
   Performance counter stats for './bin/llvm-xray convert -sort -symbolize -instr_map=./bin/llvm-exegesis -output-format=trace_event -output=/tmp/trace.yml xray-log.llvm-exegesis.lwZ0sT' (9 runs):
  
             7178.38 msec task-clock                #    1.000 CPUs utilized            ( +-  0.26% )
                 182      context-switches          #   25.402 M/sec                    ( +- 28.84% )
                   0      cpu-migrations            #    0.046 M/sec                    ( +- 70.71% )
               33701      page-faults               # 4694.994 M/sec                    ( +-  0.88% )
         28761053971      cycles                    # 4006833.933 GHz                   ( +-  0.26% )  (83.32%)
          2028297997      stalled-cycles-frontend   #    7.05% frontend cycles idle     ( +-  1.61% )  (83.32%)
         10773154901      stalled-cycles-backend    #   37.46% backend cycles idle      ( +-  0.38% )  (33.36%)
         36199132874      instructions              #    1.26  insn per cycle         
                                                    #    0.30  stalled cycles per insn  ( +-  0.03% )  (50.02%)
          6434504227      branches                  # 896420204.421 M/sec               ( +-  0.03% )  (66.68%)
            73355176      branch-misses             #    1.14% of all branches          ( +-  1.46% )  (83.33%)
  
              7.1807 +- 0.0190 seconds time elapsed  ( +-  0.26% )

So using `llvm::json` nearly triples run-time on that test case.
(+3x is times, not percent.)

Memory:
Old:

  total runtime: 39.88s.
  bytes allocated in total (ignoring deallocations): 79.07GB (1.98GB/s)
  calls to allocation functions: 33267816 (834135/s)
  temporary memory allocations: 5832298 (146235/s)
  peak heap memory consumption: 9.21GB
  peak RSS (including heaptrack overhead): 147.98GB
  total memory leaked: 1.09MB

New:

  total runtime: 17.42s.
  bytes allocated in total (ignoring deallocations): 5.12GB (293.86MB/s)
  calls to allocation functions: 21382982 (1227284/s)
  temporary memory allocations: 232858 (13364/s)
  peak heap memory consumption: 350.69MB
  peak RSS (including heaptrack overhead): 2.55GB
  total memory leaked: 79.95KB

Diff:

  total runtime: -22.46s.
  bytes allocated in total (ignoring deallocations): -73.95GB (3.29GB/s)
  calls to allocation functions: -11884834 (529155/s)
  temporary memory allocations: -5599440 (249307/s)
  peak heap memory consumption: -8.86GB
  peak RSS (including heaptrack overhead): 0B
  total memory leaked: -1.01MB

So using `llvm::json` increases *peak* memory consumption on *this* testcase ~+27x.
And total allocation count +15x. Both of these numbers are times, *not* percent.

And note that memory usage is clearly unbound with `llvm::json`, it directly depends
on the length of the log, so peak memory consumption is always increasing.
This isn't so with the dumb code, there is no accumulating memory consumption,
peak memory consumption is fixed. Naturally, that means it will handle *much*
larger logs without OOM'ing.

Readability is good, but the price is simply unacceptable here.
Too bad none of this analysis was done as part of the development/review D50129 <https://reviews.llvm.org/D50129> itself.


Repository:
  rL LLVM

https://reviews.llvm.org/D58584

Files:
  tools/llvm-xray/xray-converter.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D58584.188059.patch
Type: text/x-patch
Size: 6477 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190223/98f956a3/attachment-0001.bin>


More information about the llvm-commits mailing list