BenchmarkTools.jl icon indicating copy to clipboard operation
BenchmarkTools.jl copied to clipboard

Measure cpu-time and real-time and report both

Open vchuravy opened this issue 7 years ago • 25 comments

This still needs some cleanup and the history is a bit messy. In essence this measures both the time actually spend running on the cpu and the time passed. If the OS migrates a process the cpu-time won't increase, while real-time will. Similar for other things like calling into the kernel.

  • [x] Update docs

vchuravy avatar Jan 18 '18 22:01 vchuravy

The code in timers is mostly taken from google/benchmark and that is under Apache v2.0. The code is fairly basic, but in order to make sure I retained the copyright notice as mandated by the Apache License. So the whole Timers module is under Apache v2

vchuravy avatar Jan 19 '18 18:01 vchuravy

I don't think the Apache license mandates that derived works be licensed under Apache; as I understand it it's like MIT in that you just need to preserve the original license when distributing the original code (in whole or in part). So in this case I think it's worth acknowledging that the code is derived from Google stuff but I don't think it requires a separate license.

ararslan avatar Jan 19 '18 18:01 ararslan

The Apache license does mandate that derived works keep the license notice intact and include a copy of the license and mention all modifications...

"Derivative Works" shall mean any work, whether in Source or Object form, that is based on (or derived from) the Work and for which the editorial revisions, annotations, elaborations, or other modifications represent, as a whole, an original work of authorship. For the purposes of this License, Derivative Works shall not include works that remain separable from, or merely link (or bind by name) to the interfaces of, the Work and Derivative Works thereof.

a; You must give any other recipients of the Work or Derivative Works a copy of this License; and b; You must cause any modified files to carry prominent notices stating that You changed the files; and c; You must retain, in the Source form of any Derivative Works that You distribute, all copyright, patent, trademark, and attribution notices from the Source form of the Work, excluding those notices that do not pertain to any part of the Derivative Works; and

So yeah, I would go for the conservative option and consider a translation of code a derivative work.

vchuravy avatar Jan 19 '18 19:01 vchuravy

Right, we just need to include or link to their license, we don't have to license our own under Apache.

ararslan avatar Jan 19 '18 19:01 ararslan

That is contrarian to my understanding of the license.

I translated code under the Apache license to Julia, therefore creating a derivative work. According to the license I must a; keep the license notice, b; include a prominent notice that I changed the files and c; I must give any recipient a copy of the license.

So the code under src/timers is Apache licensed and everything else is MIT.

vchuravy avatar Jan 19 '18 21:01 vchuravy

It's not a copyleft license; derivative works don't need to bear the same license. See https://www.whitesourcesoftware.com/whitesource-blog/top-10-apache-license-questions-answered/.

ararslan avatar Jan 19 '18 21:01 ararslan

Ok, changed the license to MIT, while complying with the Apache requirement of retaining the license notices. This is pretty much ready to merge from my side.

vchuravy avatar Jan 19 '18 22:01 vchuravy

I would appreciate if people on arcane platforms like Windows or Mac could give this I try. This should work on more reasonable platforms like a current FreeBSD. But I personally have only tested this on Linux 64bit.

vchuravy avatar Jan 19 '18 22:01 vchuravy

Let's hold merging this for now. I got a report that this reports odd things on Windows.

vchuravy avatar Jan 19 '18 23:01 vchuravy

Confirmed that tests pass on current Julia master on FreeBSD 11.1.

ararslan avatar Jan 20 '18 00:01 ararslan

Ok hunted down the Windows issue and updated the docs. It turns out the accuracy of the windows counters is only 100ns and so the information becomes fairly useless for short functions. Jarrett and I discussed a variety of possibilities to deal with this, but in the end we decided to simply turn off printing for cpu-time on Windows.

vchuravy avatar Jan 23 '18 01:01 vchuravy

I was informed that I missed https://msdn.microsoft.com/en-us/library/windows/desktop/ms684929(v=vs.85).aspx (https://msdn.microsoft.com/en-us/library/windows/desktop/dn553408(v=vs.85).aspx) and so there is hope that we get accurate timings on windows after all.

vchuravy avatar Jan 23 '18 15:01 vchuravy

So I am currently blocked on converting the cycles to a reasonable time.

  • tsc_invariant/tsc_constant are necessary http://oliveryang.net/2015/09/pitfalls-of-TSC-usage/ tsc_invariant is on Nehalem-and-later
  • Getting the TSC frequency is tricky.
  • Reading MSR_PLATFORM_INFO [15:8] through the rdmsr instruction, which can't be executed in user-space.
  • Reading through the Intel Manual I noticed that there is a section for cpuid Time Stamp Counter and Nominal Core Crystal Clock Information Leaf which has the necessary information sometimes...
function cpuid(infoType)
         info = Ref{NTuple{4, UInt32}}()
         ccall(:jl_cpuid, Cvoid, (Ref{NTuple{4, UInt32}}, UInt32), info, infoType)
         info[]
end
tsc_invariant() = cpuid(0x80000007)[4] == 0x00000100
function tsc_freq()
          info = cpuid(0x15)
          if info[1] == 0 || info[3] == 0
            return nothing
          else
             return info[2] / info[1] * info[3]
          end
end

and on my machine info[3] == 0 The manual specifies:

TSC_Value = (ART_Value * CPUID.15H:EBX[31:0] )/ CPUID.15H:EAX[31:0] + K

vchuravy avatar Jan 24 '18 22:01 vchuravy

So I am currently blocked on converting the cycles to a reasonable time.

We can always merge this without the Windows-specific improvements (and just print N/A or w/e for cpu-time on Windows), and include the improvements in a future PR once they're ready.

jrevels avatar Jan 25 '18 16:01 jrevels

I have to double check that the frequency of QueryProcessCycleTime and QueryPerformanceCounter are the same, but otherwise I found a workable solution for Windows.

I will let this stew for a bit and focus other work.

vchuravy avatar Jan 25 '18 17:01 vchuravy

This feature is very exciting. Is there any technical issue that is preventing this PR from being merged?

It seems even as far as CPU Time goes, there may be multiple measures in certain OSes (e.g. https://linux.die.net/man/2/getrusage mentions RUSAGE_THREAD) for measuring per-thread times, too, which might suggest there's room for more features in future PRs.

goretkin avatar Jan 27 '20 20:01 goretkin

This feature is very exciting. Is there any technical issue that is preventing this PR from being merged?

I think I got annoyed at Windows, but I don't recall the details anymore.

vchuravy avatar Jan 20 '22 17:01 vchuravy