optview2 icon indicating copy to clipboard operation
optview2 copied to clipboard

Fixes for Windows

Open clemenswasser opened this issue 1 year ago • 12 comments

I've verified that this branch works on Windows, Linux, and macOS. It uses the LLVM Demangle lib as a cross-platform demangler (when merged back upstream, it should be easier to require it, as it's one of LLVM's core libraries).

clemenswasser avatar Jan 14 '23 14:01 clemenswasser

@clemenswasser Thank you very much for taking the effort to do this!

It seems the 'Demangle' sources you suggest adding are used in a tool I wasn't aware of - llvm-cxxfilt . I briefly considered demumble and maybe one other, but wasn't aware of an llvm demangling solution that applies beyond clang.

For me it is installed alongside clang (/usr/lib/llvm-14/bin). Do you know if it's installable (as binary) on Windows/macOS? If yes, I think it is better to try and use it by default, and if not available - emit a suggestion to the user to install it.

Alternatively - I know of some projects that include such external binaries they depend on in the actual repo (as binaries). Don't know yet what I think of that.

OfekShilon avatar Jan 14 '23 17:01 OfekShilon

Yes, using llvm-cxxfilt sounds ideal 👍 I've looked and llvm-cxxfilt gets installed with the LLVM installer on Windows under C:\Program Files\LLVM\bin\llvm-cxxfilt.exe by default and is in $PATH. I will update my PR shortly.

clemenswasser avatar Jan 14 '23 17:01 clemenswasser

@OfekShilon I've now changed to using llvm-cxxfilt, but it crashes (only) on Windows, do you have any idea what the problem could be?

multiprocessing.pool.RemoteTraceback:
"""
Traceback (most recent call last):
  File "C:\Program Files\Python311\Lib\multiprocessing\pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
                    ^^^^^^^^^^^^^^^^^^^
  File "C:\Program Files\Python311\Lib\multiprocessing\pool.py", line 48, in mapstar
    return list(map(*args))
           ^^^^^^^^^^^^^^^^
  File "optpmap.py", line 24, in _wrapped_func
    return func(argument, source_dir, exclude_names, exclude_text, collect_opt_success, annotate_external)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "opt-viewer.py", line 297, in _render_file
    render_file_source(source_dir, output_dir, filename, remarks)
  File "opt-viewer.py", line 143, in render_file_source
    entries = list(render_source_lines(source_stream, line_remarks))
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "opt-viewer.py", line 90, in render_source_lines
    yield render_inline_remark(remark, html_line)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "opt-viewer.py", line 101, in render_inline_remark
    inlining_context = remark.DemangledFunctionName
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "optrecord.py", line 153, in DemangledFunctionName
    return self.demangle(self.Function)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "optrecord.py", line 77, in demangle
    with cls.demangler_lock:
         ^^^^^^^^^^^^^^^^^^
AttributeError: type object 'Missed' has no attribute 'demangler_lock'
"""

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "opt-viewer.py", line 504, in <module>
    main()
  File "opt-viewer.py", line 492, in main
    generate_report(all_remarks=all_remarks,
  File "opt-viewer.py", line 355, in generate_report
    optpmap.pmap(func=_render_file_bound,
  File "optpmap.py", line 48, in pmap
    result = pool.map(_wrapped_func, func_and_args, *args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Program Files\Python311\Lib\multiprocessing\pool.py", line 367, in map
    return self._map_async(func, iterable, mapstar, chunksize).get()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Program Files\Python311\Lib\multiprocessing\pool.py", line 774, in get
    raise self._value
AttributeError: type object 'Missed' has no attribute 'demangler_lock'

clemenswasser avatar Jan 14 '23 18:01 clemenswasser

@supox maybe you can help?

OfekShilon avatar Jan 15 '23 08:01 OfekShilon

I think I can shed some light on the issue (don't have a solution yet). Basically, it is the difference between multiprocessing on Windows and Linux - Remark.demangle and Remark.demangle_locks are not shared!

https://www.pythonforthelab.com/blog/differences-between-multiprocessing-windows-and-linux/

code in PR will run fine with -j 1 switch (as no MP is available_)

Since I am not an expert in Python, I currently can not tell how to solve it, besides extremely silly and obvious solution (works for me, though obviously it creates unnecessary lock for each process, and multiple instances of demangle) :)

diff --git a/optrecord.py b/optrecord.py
index 1e131d9..a284899 100755
--- a/optrecord.py
+++ b/optrecord.py
@@ -72,6 +72,8 @@ class Remark(yaml.YAMLObject):
 
     @classmethod
     def demangle(cls, name):
+        if not cls.demangler_proc:
+            cls.set_demangler(cls.default_demangler)
         with cls.demangler_lock:
             cls.demangler_proc.stdin.write((name + '\n').encode('utf-8'))
             cls.demangler_proc.stdin.flush()

May be someone more experienced can do it better! @OfekShilon @clemenswasser

AntonYudintsev avatar Jan 17 '23 09:01 AntonYudintsev

@AntonYudintsev Thank you very much!

@YoavShilon05 perhaps you can take a look too?

OfekShilon avatar Jan 17 '23 10:01 OfekShilon

I've been testing with this PR's patches on a fairly large project built with clang-cl, and it appears to be working without too much issue.

I did need to add a patch to replace \\ in the generated HTML filenames - otherwise, the HTML rendering would fail:

2023-01-17 14:28:21,464 INFO Rendering HTML files...
        1 of 535Traceback (most recent call last):
  File "D:\optview2\opt-viewer.py", line 504, in <module>
    main()
  File "D:\optview2\opt-viewer.py", line 492, in main
    generate_report(all_remarks=all_remarks,
  File "D:\optview2\opt-viewer.py", line 355, in generate_report
    optpmap.pmap(func=_render_file_bound,
  File "D:\optview2\optpmap.py", line 42, in pmap
    result = list(map(_wrapped_func, func_and_args, *args, **kwargs))
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\optview2\optpmap.py", line 24, in _wrapped_func
    return func(argument, source_dir, exclude_names, exclude_text, collect_opt_success, annotate_external)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\optview2\opt-viewer.py", line 297, in _render_file
    render_file_source(source_dir, output_dir, filename, remarks)
  File "D:\optview2\opt-viewer.py", line 133, in render_file_source
    with open(html_filename, "w", encoding='utf-8') as f:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: 'D:\\[project]\\html\\[redacted]\\[redacted].c.html'

Here is the patch that I used.

Unfortunately, I'm not familiar enough with Python's multiprocessing lib to help fix the Windows/Unix threading issue.

abbriggs avatar Jan 17 '23 22:01 abbriggs

@abbriggs @OfekShilon

I have made a (working for me) pull request (since I can't/don't know how amend this). "No locks" is also arguably better solution on **nix platforms as well, as Locks are also not free, and if there are many files to process, it is probably better to have one demangler per process than to lock across many processes.

Anyway,

  • I have no idea how to fix it for Windows otherwise:)
  • for all other platforms right now it is just one more if per remark, and it is working on Windows.

AntonYudintsev avatar Jan 18 '23 11:01 AntonYudintsev

@clemenswasser I apologize for the delay in looking into this. I just tested and merged @AntonYudintsev's PR that I believe properly works around the MP linux/windows diff.
In the mean time also the source_dir argument was made absolute and passed down various methods, as I see you did too.

I intend also to search various demanglers (at the very least c++filt, llvm-cxxfilt and undname) at default install locations and use what is available - with fall back to non-demangling.

Could you please check if the latest version works for you on windows? (you'd probably need to still pass --demangler explicitly)

Are there other issues that are addressed in your PR? If yes - could you kindly separate it into smaller PRs (1 issue per PR) to ease reviewing and merging?

OfekShilon avatar Feb 11 '23 21:02 OfekShilon

@OfekShilon no apologies needed, thanks for merging this! @AntonYudintsev patch doesn't really work for me. When setting --demangler 'llvm-cxxfilt -n', it still uses the default_demanger: c++filt due to the multiprocessing inconsistency mentioned before. This then raises the following exception:

Failed to process file D:\dev\cpp\optview2\cpp_optimization_example\main.cc
multiprocessing.pool.RemoteTraceback:
"""
Traceback (most recent call last):
  File "C:\Users\cleme\AppData\Local\Programs\Python\Python311\Lib\multiprocessing\pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
                    ^^^^^^^^^^^^^^^^^^^
  File "C:\Users\cleme\AppData\Local\Programs\Python\Python311\Lib\multiprocessing\pool.py", line 48, in mapstar
    return list(map(*args))
           ^^^^^^^^^^^^^^^^
  File "D:\dev\cpp\optview2\optpmap.py", line 24, in _wrapped_func
    return func(argument, exclude_names, exclude_text, collect_opt_success,
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\dev\cpp\optview2\opt-viewer.py", line 297, in _render_file
    render_file_source(source_dir, output_dir, filename, remarks)
  File "D:\dev\cpp\optview2\opt-viewer.py", line 143, in render_file_source
    entries = list(render_source_lines(source_stream, line_remarks))
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\dev\cpp\optview2\opt-viewer.py", line 90, in render_source_lines
    yield render_inline_remark(remark, html_line)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\dev\cpp\optview2\opt-viewer.py", line 101, in render_inline_remark
    inlining_context = remark.DemangledFunctionName
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\dev\cpp\optview2\optrecord.py", line 176, in DemangledFunctionName
    return self.demangle(self.Function)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\dev\cpp\optview2\optrecord.py", line 99, in demangle
    cls.set_demangler_no_lock(cls.default_demangler)
  File "D:\dev\cpp\optview2\optrecord.py", line 93, in set_demangler_no_lock
    cls.set_base_demangler(demangler)
  File "D:\dev\cpp\optview2\optrecord.py", line 84, in set_base_demangler
    cls.demangler_proc = subprocess.Popen(demangler.split(), stdin=subprocess.PIPE, stdout=subprocess.PIPE)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\cleme\AppData\Local\Programs\Python\Python311\Lib\subprocess.py", line 1024, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "C:\Users\cleme\AppData\Local\Programs\Python\Python311\Lib\subprocess.py", line 1493, in _execute_child
    hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [WinError 2] The system cannot find the file specified

clemenswasser avatar Feb 11 '23 21:02 clemenswasser

@clemenswasser can you please share the exact steps you take to build cpp_optimization_example and run optview2 on it over windows? I hope to reproduce and investigate.

OfekShilon avatar Feb 11 '23 21:02 OfekShilon

Sure, this reproduces the exception in PowerShell:

> clang++ -fsave-optimization-record '-foptimization-record-file=main.yaml' -O3 --std=c++17 main.cc
> python ..\opt-viewer.py --demangler 'llvm-cxxfilt.exe -n' .\main.yaml

clemenswasser avatar Feb 11 '23 22:02 clemenswasser