coala: Add file caching and remove Tagging #2016

adtac · 2016-04-14T18:39:27Z

With --caching the user can run coala only on those files that
had changed since the last time coala was run. This should improve
the running time of coala.

sils · 2016-04-14T19:58:31Z

@fneu please take a look ASAP

fneu · 2016-04-14T20:17:04Z

coalib/misc/Caching.py

+import calendar
+import time
+import appdirs
+import pickle


Hm, I'm not completely sold on pickle, yet. It's nice that until now we have resided with human-readable files, although I get that pickle makes things a little easier in this case. And this is something that peolpe would most likely not interfere with besides deleting said file in case they suspect it leading to errors.

@hypothesist what happens if coala is interrupted while writing to the pickled file? Will this break things (and therefore permanently break coala until a human intervenes?)

^@sils1297

On CTRL+C pickle ignores it and writes everything to file. If you kill the process its mostly corrupted^^
In this case we just need to catch UnpicklingError^^

@fneu I think it will, at worst, corrupt the file. I didn't think about the possibility of that happening.

So I think, as @Makman2 said, adding a UnpickingError should suffice. The worst that will happen in that case is that every project will need to be cached again (maybe we can display an error saying caching broke, so every project needs to be cached again). I don't think that's too bad.

Yeah I think that's the way forward. Post a warning message, delete the
corrupt file, re-cache.

All in all I like this very much :)

Am 15. April 2016 9:02:27 vorm. schrieb Adhityaa Chandrasekar
notifications@github.com:

@@ -0,0 +1,142 @@
+import os
+import calendar
+import time
+import appdirs
+import pickle

@fneu I think it will, at worst, corrupt the file. I didn't think about the
possibility of that happening.

So I think, as @Makman2 said, adding a UnpickingError should suffice. The
worst that will happen in that case is that every project will need to be
cached again (maybe we can display an error saying caching broke, so every
project needs to be cached again). I don't think that's too bad.

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
https://github.com/coala-analyzer/coala/pull/2016/files/621fc8d315bd5a587846946475b6ce9370489585#r59833966

@fneu I've added a verify_file_integrity function that will check if all files are proper. It will delete all of them if even one of them is corrupted (doesn't make sense to have the others if they are all connected). This is done at the start of each coala execution (before the first time any pickling is done).

sounds fine to me

for me too 👍

fneu · 2016-04-14T20:26:37Z

this actually does look rather sane!

there are still code paths not covered by tests, please complete them

fneu · 2016-04-14T20:27:39Z

@hypothesist you write: "This should improve the running time of coala". I'm on windows right now and cannot check: Is there a run time decrease and how big is the improvement?

Makman2 · 2016-04-14T21:07:30Z

@fneu depending on how many files were changed. Last time @hypothesist told me he was 66% faster^^

adtac · 2016-04-15T04:54:54Z

@fneu @Makman2 is right, it was 3x faster.

tushar-rishav · 2016-04-15T08:49:12Z

coalib/misc/Caching.py

@@ -0,0 +1,171 @@
+import os


@hypothesist Perhaps we should have it alphabetically sorted.

tushar-rishav · 2016-04-15T09:23:36Z

@hypothesist 1c45bd7 needs minor rework

tushar-rishav · 2016-04-15T14:41:18Z

coalib/processes/Processing.py

@@ -423,6 +436,22 @@ def yield_ignore_ranges(file_dict):
                                               len(file[line_number])))


+def get_file_list(results):
+    """
+    Gets the list of files that are affected in the given results.


@hypothesist You just missed this 😉

Fixed it. In my defense, the whole file uses imperative! :P

sils · 2016-04-17T16:30:17Z

coalib/misc/Caching.py

+                     user's config directory.
+    """
+    path = appdirs.user_config_dir("coala")
+    return os.path.join(path, filename)


don't we have a function like this in tagging somewhere?

sils · 2016-06-12T09:27:57Z

coalib/output/dbus/DbusDocument.py

@@ -132,6 +132,7 @@ def Analyze(self):
                        global_bear_list=global_bears[section_name],
                        local_bear_list=local_bears[section_name],
                        print_results=lambda *args: True,
+                        cache=None,


you should at least add a TODO or something

sils · 2016-06-12T09:37:02Z

unack 611bee3

sils · 2016-06-12T10:55:51Z

ack 7abfbbb

sils · 2016-06-12T10:57:44Z

coalib/processes/Processing.py

@@ -470,6 +509,8 @@ def process_queues(processes,
                               filename as keys.
    :param print_results:      Prints all given results appropriate to the
                               output medium.
+    :param cache:              An instance of ``misc.Caching.FileCache`` to use
+                               as a file cache buffer.


file a newcomer issue about this being allowed None

sils · 2016-06-12T10:58:22Z

ack 0bc76fb#diff-7c16cc0dc5d9e615bbb54239955f7b5eR104

sils · 2016-06-12T10:59:40Z

ack 7aa7c3c

This commit introduces helper functions that will be used in a later commit for caching (running coala only on changed files).

In this commit, coalib.misc.Caching is introduced which contains a class Cache which contains methods to act as a caching mechanism that will be useful to determine which files are new or changed and run coala only on those files to improve coala run time speed.

With `--changed-files` the user can run coala only on those files that had changed since the last time coala was run. This should improve the running time of coala. Fixes #1991

adtac · 2016-06-12T11:58:49Z

@sils1297 sorry, I amended the previous commits by habit.

I'll detail the changes I made so that it's easier to review:

622a3e5 [Checking if file is present] [Removing the settings field from the settings db] [Test permission errors]
5a6870c [Fixed the flush_cache condition]
91c109d [Update the settings db irrespective of whether caching is enabled] - this is done because if the user runs coala with caching once and then never uses caching again, it would always keep flagging the settings change since it isn't getting updated (that's what happened in your case).

sils · 2016-06-12T12:17:50Z

ack 622a3e5

sils · 2016-06-12T12:19:24Z

ack 5a6870c#diff-428b725e2ad8494e5b87efe77b1efe1cR96

sils · 2016-06-12T12:19:48Z

ack 91c109d

sils · 2016-06-12T12:19:54Z

ack ad3ec72

adtac · 2016-06-12T12:20:35Z

@rultor merge

rultor · 2016-06-12T12:21:24Z

@rultor merge

@hypothesist OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2016-06-12T12:22:55Z

@rultor merge

@hypothesist Done! FYI, the full log is here (took me 1min)

adtac added the process/wip label Apr 14, 2016

adtac force-pushed the hypothesist/wip branch 2 times, most recently from 6d68870 to bac2c36 Compare April 14, 2016 19:30

fneu reviewed Apr 14, 2016
View reviewed changes

adtac force-pushed the hypothesist/wip branch 2 times, most recently from 914e6b3 to dd21d95 Compare April 15, 2016 08:06

tushar-rishav reviewed Apr 15, 2016
View reviewed changes

adtac force-pushed the hypothesist/wip branch from dd21d95 to f9afb6b Compare April 15, 2016 13:47

gitmate-bot added process/pending review size/XL cautious review and removed process/wip labels Apr 15, 2016

adtac force-pushed the hypothesist/wip branch from f9afb6b to 477d0fe Compare April 15, 2016 13:58

tushar-rishav reviewed Apr 15, 2016
View reviewed changes

adtac force-pushed the hypothesist/wip branch from 477d0fe to cab9cee Compare April 16, 2016 04:01

gitmate-bot added the review carefully! label Apr 16, 2016

adtac force-pushed the hypothesist/wip branch from cab9cee to 212586f Compare April 16, 2016 04:02

sils removed the cautious review label Apr 16, 2016

sils reviewed Apr 17, 2016
View reviewed changes

sils reviewed Jun 12, 2016
View reviewed changes

sils added process/wip and removed process/pending review labels Jun 12, 2016

adtac force-pushed the hypothesist/wip branch from 611bee3 to 4fc5816 Compare June 12, 2016 10:23

gitmate-bot added process/pending review and removed process/wip labels Jun 12, 2016

adtac force-pushed the hypothesist/wip branch from 4fc5816 to 0bc76fb Compare June 12, 2016 10:47

sils reviewed Jun 12, 2016
View reviewed changes

adtac added 4 commits June 12, 2016 17:12

Add caching utilities

622a3e5

This commit introduces helper functions that will be used in a later commit for caching (running coala only on changed files).

Add option to run coala only on changed files

91c109d

With `--changed-files` the user can run coala only on those files that had changed since the last time coala was run. This should improve the running time of coala. Fixes #1991

coalaCITest: Remove unused imports

ad3ec72

adtac force-pushed the hypothesist/wip branch from 0bc76fb to ad3ec72 Compare June 12, 2016 11:49

sils added process/approved The PR is approved and will be merged soon and removed process/pending review labels Jun 12, 2016

rultor merged commit ad3ec72 into master Jun 12, 2016

sils deleted the hypothesist/wip branch June 12, 2016 13:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

coala: Add file caching and remove Tagging #2016

coala: Add file caching and remove Tagging #2016

adtac commented Apr 14, 2016

sils commented Apr 14, 2016

fneu Apr 14, 2016

fneu Apr 14, 2016

Makman2 Apr 14, 2016

adtac Apr 15, 2016

fneu Apr 15, 2016

adtac Apr 15, 2016

sils Apr 15, 2016

Makman2 Apr 15, 2016

fneu commented Apr 14, 2016

fneu commented Apr 14, 2016

Makman2 commented Apr 14, 2016

adtac commented Apr 15, 2016

tushar-rishav Apr 15, 2016

tushar-rishav commented Apr 15, 2016

tushar-rishav Apr 15, 2016

adtac Apr 16, 2016

sils Apr 17, 2016

sils Jun 12, 2016

sils commented Jun 12, 2016

sils commented Jun 12, 2016

sils Jun 12, 2016

sils commented Jun 12, 2016

sils commented Jun 12, 2016

adtac commented Jun 12, 2016

sils commented Jun 12, 2016

sils commented Jun 12, 2016

sils commented Jun 12, 2016

sils commented Jun 12, 2016

adtac commented Jun 12, 2016

rultor commented Jun 12, 2016

rultor commented Jun 12, 2016

coala: Add file caching and remove Tagging #2016

coala: Add file caching and remove Tagging #2016

Conversation

adtac commented Apr 14, 2016

sils commented Apr 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fneu commented Apr 14, 2016

fneu commented Apr 14, 2016

Makman2 commented Apr 14, 2016

adtac commented Apr 15, 2016

Choose a reason for hiding this comment

tushar-rishav commented Apr 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sils commented Jun 12, 2016

sils commented Jun 12, 2016

Choose a reason for hiding this comment

sils commented Jun 12, 2016

sils commented Jun 12, 2016

adtac commented Jun 12, 2016

sils commented Jun 12, 2016

sils commented Jun 12, 2016

sils commented Jun 12, 2016

sils commented Jun 12, 2016

adtac commented Jun 12, 2016

rultor commented Jun 12, 2016

rultor commented Jun 12, 2016