Code Review: Added logTime to the Debug module


#1

I’ve made a PR adding logTime to the Debug module in the elm/core package.

In 0.18 I made a native module with this and I found it quite useful when tracking down rendering performance issues. Since native modules are not possible in 0.19 I decided to try a PR instead.

I’m asking for feedback on mainly four things:

  • Is the implementation correct? This is my first time messing with native js in 0.19
  • Is the documentation OK? (English is not my native language).
  • Is there a better name than Debug.logTime?
  • Would you also find this useful?

Of course, any other feedback is also welcome.

Thanks


#2

I would definitely find it useful, and I think that such a function is an essential part of debugging tools. Just few days ago I had a case where my app was having 500ms delays, and I was hoping that function like this would exist. (While I knew quite well where the problem was, I would’ve liked to time some specific parts to get more details.)

While elapsed time is most useful here, I’d prefer function to also output current time with millisecond resolution, so I can compare starting times of different parts of code.


#3

Why not Debug.currentTime : Time.Posix or similar so you can do some calculations over your timestamps? You could still print that value.

EDIT: oh, it’s measuring the time taken to execute the wrapped function. Seems a bit vulnerable to js optimisations, but ok. Go for Debug.measureTime : (() -> b) -> (b, Time.Posix) then, or something similar.


#4

This topic was automatically closed 10 days after the last reply. New replies are no longer allowed.