eteran / edb-debugger

edb is a cross-platform AArch32/x86/x86-64 debugger.
GNU General Public License v2.0
2.7k stars 326 forks source link

RFC: formatting floating-point numbers in their "shortest" closest number form #665

Closed 10110111 closed 6 years ago

10110111 commented 6 years ago

These patches implement formatting of floating-point values without the ugliness of numbers like 1e-11 looking like 9.9999999999999994e-12. The cost of this is introduction of two optional dependencies to EDB.

Now, this PR is an RFC, not intended to be merged as is. The gdtoa repo is even in a form that might be not best to reference (e.g. it might be a good idea to give it a bit different name to avoid confusion with the original gdtoa (which it does contain though)). I've not updated the README, and have a few questions/issues to discuss.

  1. What is the general feeling of adding two new dependencies? Is it OK?
  2. Using double-conversion when gdtoa seems to be enough may seem redundant. But the reason I did this nonetheless is that double-conversion is several times faster than gdtoa, especially in the more common (for a debugger) case of very small and very large values (e.g. XMM0 register containing a vector of small dwords would look like containing denormals in float view). Since EDB may need to do quite many conversions on each single-step due to SIMD registers, performance doesn't look like a negligible concern.
  3. It feels a bit incomplete that we require double-conversion to nicely format floats and doubles, although gdtoa is capable of this too. It might be a good idea to use gdtoa for all the types if double-conversion is for some reason unavailable, while gdtoa is available.

For reference, here're my performance measurements on Core i7-4765T:

┌──────┬─────────────────────────────┬──────────────────────────────────────────────────┐
│      │   Time per conversion,μs    │                  Time ratio                      │
├──────┼─────────┬──────────┬────────┼───────────────┬───────────────┬──────────────────┤
│stdDev│ gdtoa   │doub-conv │snprintf│gdtoa/doub-conv│gdtoa/snprintf │snprintf/doub-conv│
├──────┼─────────┼──────────┼────────┼───────────────┼───────────────┼──────────────────┤
│1e-310│ 11.8573 │ 2.77513  │4.09507 │    4.27269    │    2.89549    │     1.47564      │
│1e-300│ 13.1082 │ 1.7818   │3.11804 │    7.35671    │    4.20399    │     1.74994      │
│1e-200│ 10.6658 │ 1.70984  │2.72838 │    6.2379     │    3.90922    │     1.59569      │
│1e-100│ 8.00228 │ 1.71608  │2.39582 │    4.66311    │    3.3401     │     1.3961       │
│1e-50 │ 6.52289 │ 1.70504  │2.23901 │    3.82566    │    2.91329    │     1.31318      │
│1     │ 4.95917 │ 1.67482  │1.91259 │    2.96102    │    2.59291    │     1.14197      │
│100000│ 4.9963  │ 1.75312  │2.40833 │    2.84995    │    2.07459    │     1.37374      │
│1e+10 │ 4.97863 │ 1.68108  │2.40476 │    2.96156    │    2.07032    │     1.43049      │
│1e+14 │ 5.07853 │ 1.88202  │2.49318 │    2.69845    │    2.03697    │     1.32474      │
│1e+16 │ 5.11652 │ 2.10139  │2.41447 │    2.43483    │    2.11911    │     1.14899      │
│1e+17 │ 5.09547 │ 2.52605  │2.38239 │    2.01717    │    2.13881    │     0.94313      │
│1e+18 │ 5.0824  │ 1.93073  │2.4469  │    2.63237    │    2.07708    │     1.26734      │
│1e+20 │ 5.1102  │ 1.76286  │2.48284 │    2.89882    │    2.05821    │     1.40842      │
│1e+80 │ 6.47925 │ 1.72396  │3.07891 │    3.75835    │    2.1044     │     1.78595      │
│1e+150│ 8.27003 │ 1.72629  │3.47059 │    4.79064    │    2.38289    │     2.01043      │
│1e+250│ 11.4804 │ 1.72706  │4.45732 │    6.64734    │    2.57562    │     2.58087      │
│1e+308│ 11.4766 │ 1.62947  │4.18172 │    7.04315    │    2.74447    │     2.56631      │
└──────┴─────────┴──────────┴────────┴───────────────┴───────────────┴──────────────────┘
* stdDev is standard deviation of normally distributed zero-centered test random
  double-precision numbers
eteran commented 6 years ago

Interesting, I'll have to take a close look at this.

In general, I am always in favor of optional dependencies. As in, things will work just the same if the user doesn't have the dep, but if they happen to have it installed, things will work better automagically.

10110111 commented 6 years ago

Hi, any news on this?

eteran commented 6 years ago

Sorry, I didn't realize you were waiting for further input.

My understanding is that this PR was not intended to be merged but was simply an RFC, much about adding new dependencies.

In general and in this case I support adding new dependencies, I would just like to make sure that we have a graceful fall back for when the dependencies aren't there.

Of course, I am sure that there are cases where a new feature simply requires a dependency. I figure we'll tackle those on a case-by-case basis.

10110111 commented 6 years ago

Yeah, the point 1. had been answered by your first comment. I'm now mainly interested in your opinion on points 2. and 3..

eteran commented 6 years ago

I think whatever you feel would result in the highest quality code/results is fine by me 👍