Run gtm in background with rate limit of 1/s

git-time-metric / gtm-jetbrains-plugin

IntelliJ IDEA, PyCharm, WebStorm, AppCode, RubyMine, PhpStorm, AndroidStudio Plugins

MIT License

37 stars 7 forks source link

Run gtm in background with rate limit of 1/s #6

Closed wACKYz closed 4 years ago

wACKYz commented 6 years ago

Fixes issues with lagging tab switching. With status enabled UI was locked till end of GTM execution.

mschenk42 commented 6 years ago

Hi @wACKYz, GTM should be very fast and not noticeable when recording. We shouldn't have to run it in the background. It could be either something local to your computer and/or configuration or a bug in GTM or the JetBrain's PlugIn that needs to be addressed.

Can you provide some more information on the issue? What would be helpful is to add logging to plugin for how long the gtm record takes.

wACKYz commented 6 years ago

Hi,

On OSX 2.7GHz i5 (2cores with HT) local encrypted drive

for x in {1..10} ; do \time /usr/local/bin/gtm record --status /Users/.... ; done
11m0s        0.26 real         0.06 user         0.09 sys
11m0s        0.11 real         0.05 user         0.06 sys
11m0s        0.13 real         0.04 user         0.05 sys
11m0s        0.10 real         0.04 user         0.05 sys
11m0s        0.16 real         0.04 user         0.05 sys
11m0s        0.09 real         0.03 user         0.05 sys
11m0s        0.10 real         0.04 user         0.05 sys
11m0s        0.12 real         0.04 user         0.06 sys
11m0s        0.11 real         0.04 user         0.05 sys
11m0s        0.16 real         0.03 user         0.05 sys

GTM plugin with status bar enabled, waits for command to complete. So it is freezing my UI for 100ms, it means that using keyboard shortcut to switch editor tabs I'm not able to do it quickly (for e.g. move by 4 will take more than half second).

No difference on Windows machines.

IMHO, you should not make User Interface fluidity depend on executing external binary.

mschenk42 commented 6 years ago

@wACKYz I agree async makes sense when it comes to the UI. I just hadn't noticed any delay but I guess that can vary based on your setup. I was hoping for a simpler change. I'm not a Java expert and I'm sure your code is good. However I think I found a less intrusive approach to making it async. Can you take a look and let me know what you think? https://github.com/git-time-metric/gtm-jetbrains-plugin/pull/7/files

wACKYz commented 6 years ago

I'm not java specialist either :)

It create new problem:

it is possible to have more than 1 process in background, more problems appear:
race condition: no guarantee that gtm executable will write .event file in execution order (it will depend on os scheduling)

GTM use 1 second resolution. It make no sence to execute it more frequently. Simple approach would be to check if last run was less than a second ago but it is more important to keep last event than first (occured in same second). That is how I ended with my solution and I think it is one of simplest to solve it.

mschenk42 commented 6 years ago

@wACKYz I think we may be OK. I made some updates to assure it's only running one task at a time. I also synchronize the methods, so I don't think it's vulnerable to a race condition.

It is true that GTM does track time spent in seconds but it does this without having to record every second. Checkout how the algorithm for tracking time works here https://github.com/git-time-metric/gtm/wiki/Time-Tracking-Algorithm.

wACKYz commented 6 years ago

@mschenk42

Locks UI: at least on OSX. Try to put sleep in GTM executable.
error case: look at file A then switch to file B immediately (in same second, before GTM record A will finish) then stare at file B. It will be counted as time spend on file A
resource overuse: it will execute GTM more often that it is necesarry, draining battery

mschenk42 commented 6 years ago

@wACKYz so far I'm not seeing any of those issues. I'm running macOS Sierra with IntelliJ IDEA 2017.2.

wACKYz commented 6 years ago

@mschenk42

IntelliJ IDEA 2017.2.6 Build #IU-172.4574.11, built on November 13, 2017 JRE: 1.8.0_152-release-915-b12 x86_64 JVM: OpenJDK 64-Bit Server VM by JetBrains s.r.o Mac OS X 10.12.6

replaced gtm with:

#!/bin/sh
sleep 10

changed permissions to 755 ... and ... IDE is not responding

mschenk42 commented 6 years ago

@wACKYz I see what you mean now. Thank you. That's a nice way to verify it's doing what you expect. It was still running synchronously. I shouldn't be synchronizing the runRecord method. I only need that for the submitRecord. I tested with calling the sleep process and the IDE no longer pauses. I pushed the commits to master.