auto-compilation and visit-compiled-snippets features

[mattfidler]

See Pull #261

[mattfidler]

My use case is a slow USB drive which I run emacs on both windows and mac. On the mac the usb-drive access is fast, but on the PC it is slow. Hence, a compilation greatly speeds up my initial load time.

I would like it included. It does not have to be on by default.

[joaotavora]

Auto-compilation

Regarding 565dc5c, I agree it has to be customizable. And maybe the condition for re-compiling the directory could be made a little broader: regenerate and overwrite .yas-compiled-snippets.el (and .yas-compiled-snippets.elc) if its modification time is older than the newest snippet file in that directory.

This has the advantages:

• It is safer. If a user drops a snippet using the file manager, a reasonable use case, on next snippet load the system will generate and immediately load the correct .yas-compiled-snippets.el file.
• It is cleaner because it removes the need for the a4632ac commit (the one where you delete .yas-compiled-snippets at snippet writing time)

The disadvantage:

• It again involves stat-ing all the files in the directory, which, despite easily done with file-attributes, may be slow, punishing load times every time. I think it would nevertheless be faster than actually loading each file into a buffer and reading it (i.e not using jit).

We would have to have some benchmarks, measuring load times for small, medium and big snippet collections, under with the following conditions.

1. Liberal auto-compilation (the user is responsible for recompiling if he added some new nsnippet), use jit
2. Safe auto-compilation (the mtime mechanism), use jit
3. No auto-compilation and using jit.
4. No auto-compilation and no jit

1, 2 and 3. could be different settings of the customization variable you propose. 4 is what we had before I implemented jit happened, should be the slowest.

Using the file field in the yas-compiled-snippets files

Regarding c4b310a, I have even more doubts: on a first look, I would keep the file field as nil, because I do not want to lose the advantage of passing around .yas-compiled-snippet.el files between computers or moving your .yas-snippet-dirs. Also I'd rather not crowd yasnippet with even more customization options.

However, if we store a relative path (just the snippet file name) to the directory where the .yas-compiled-snippets the problem is avoided. When loading the yas-compiled-snippets, if we find a relative pathname, we store the correct absolute file name in memory.

In this case it would not be made configurable.

Related to c4b310a, I would also like is the possibility that, if the file isn't found (or the field is indeed nil on purpose) we can regenerate the snippet buffer and set that in snippet-mode from the info present in memory. Then, when loading, offer to save it the first position of yas/snippet-dirs as with new snippets.

[joaotavora]

Regarding auto-compilation, I cherry-picked your change into a new auto-compilation branch, reworked it a bit, and ran benchmarks (see yasnippet-benchmarks.el in that branch. This is what I got:

JIT    NO-COMPILED: 0.020363 s
JIT       COMPILED: 0.020772 s
NO-JIT NO-COMPILED: 6.318065 s
NO-JIT    COMPILED: 3.507434 s (no recompilation-needed, no auto-compilation checks)
NO-JIT    COMPILED: 3.995699 s (no recompilation needed, auto-compilation checks)
NO-JIT NO-COMPILED: 6.989287 s (recompilation needed, auto-compilation kicks in all dirs)

The penultimate case, checking for mtimes, is only a bit slower than the previous one, so I'm thinking of merging it after a few more tests.

[mattfidler]

I partially agree with your assessment. However, I think you also have to consider files that are deleted. The only way I can think of doing that is to list all the files, with their modification times, and take a MD5 of that. Save that checksum and then compare the calculated checksum to see if the snippets need to be recompiled.

I also added a bug fix for loading snippets and compiling snippets (bea8b79) without this, you cannot compile a snippet directory without any sub-directories.

[mattfidler]

I would benchmark these results, but unfortunately, I don't have access to the dictionary.

[joaotavora]

You're right about deleted snippets. While the md5 idea is very ingenious, it is not the only way. This sounds easier and faster: we can first check the mtime of the directory itself, which is updated when you delete or add a file. We recompile immediately if it happens later than .yas-compiled-snippets mtime. Otherwise we proceed as usual (checking the mtimes of the files inside it).

You don't need that dictionary file any old file of words will do...

Very important: regarding version control, please note that I am developing this in the auto-compilation branch (https://github.com/capitaomorte/yasnippet/tree/auto-compilation) so try to make changes against that. You should git merge upstream/auto-compilation into your jit-cache branch and keep most of my changes. (assuming you named my remote upstream in your config). Then work from there.

Unfortunately, according to http://stackoverflow.com/questions/10081053/how-to-change-the-base-branch-of-a-pull-request, though, you'd have to open a new pull request. I prefer to keep the discussion here, if there are any commits worth git cherry-pick'ing I'll merge them and you are given full credit as the "Author" of the commit. See https://github.com/capitaomorte/yasnippet/commits/auto-compilation for an example.

[mattfidler]

I tested your theory on windows. On windows XP deleting a file on a FAT32 drive does not change the modification time of the folder. Therefore without the checksum, the snippets are not updated correctly.

[mattfidler]

See Issue #264

[mattfidler]

I kept in the MD5 checksum since deleting files in a snippet directory didn't change the mtime of the directory. in Windows XP

[mattfidler]

Added small changes so this works on my setup. note that your setup doesn't load directories properly. If you try to load for example snippets/emacs-lisp-mode, it won't load anything. Fix bea8b79 still needs to be applied.

[mattfidler]

Using my hunspell dictionary list still doesn't work for benchmarks since (yas/with-snippet-dirs is undefined.

[joaotavora]

Hi Matthew,

So I went ahead and enhanced my version with your md5-checking idea in the auto-compilation branch. It's the same concept but it's faster. These are the results I get with your jit-cache branch (btw I fixed yasnippet-benchmarks.el)

JIT    NO-COMPILED NO-AUTOCOMP: 0.019962 s
JIT       COMPILED NO-AUTOCOMP: 0.020439 s (identical to the previous case, no snippets are actually loaded)
NO-JIT NO-COMPILED NO-AUTOCOMP: 6.238662 s (every snippet loaded from file)
NO-JIT    COMPILED NO-AUTOCOMP: 3.486777 s (no recompilation-needed)
NO-JIT    COMPILED    AUTOCOMP: 7.709266 s (no recompilation needed, just compilation checks)
NO-JIT NO-COMPILED    AUTOCOMP: 7.441429 s (recompilation needed, auto-compilation kicks in all dirs)

These are the results I get in my auto-compilation branch.

JIT    NO-COMPILED NO-AUTOCOMP: 0.020102 s
JIT       COMPILED NO-AUTOCOMP: 0.020472 s (identical to the previous case, no snippets are actually loaded)
NO-JIT NO-COMPILED NO-AUTOCOMP: 6.227525 s (every snippet loaded from file)
NO-JIT    COMPILED NO-AUTOCOMP: 3.35672 s (no recompilation-needed)
NO-JIT    COMPILED    AUTOCOMP: 3.740947 s (no recompilation needed, just compilation checks)
NO-JIT NO-COMPILED    AUTOCOMP: 9.599552 s (recompilation needed, auto-compilation kicks in all dirs)

It's quite important that the 4th and 5th cases don't differ a lot in performance, since it defeats the purpose of auto-recompilation.

The reason why your recompilation checks take so much more might be that the directory is only listed once with yas/subdirs, whereas you make more calls to yas/subdirs and directory-files. Anyway both version pass a new smart-recompilation test in yasnippet-tests.el, which I would like you to have a look at. The test considers snippet modification, additions and deletions. However your version introduced a regression in basic-jit-loading-with-compiled-snippets, which I didn't investigate.

Also, my version:

• writes the md5 hash in the .yas-compiles-snippets.el file directory, not a new file
• considers only snippet files when calculating the md5 (why would I want to consider other hidden .yas* files?)
• passes all tests in emacs22

I still don't know why you think bea8b79 is needed, since my code passes all tests. Can you write a test where it doesn't?

If you have no objections I'll close both these pull requests. Thank you for an amazing effort, I've given you credit for the idea in 66aaf61, and will do so again in the 0.7 release notes.

[mattfidler]

Thanks.

I think that by hashing out the changes the auto-compilation is better than I would have made on my own.

I do have a few comments:

• If .yas-setup.el or .yas-parents, or .yas-make-groups change, the snippet compilation should change. Therefore if these change, the compilation would be out of sync.

A use-case for this is anyone who is creating snippets that have some elisp component to it..

As far as bea8b79, I can write a test where your code doesn't pass on Windows XP. I'm not sure how to do this with ert or even how to use ert, so this is my best guess:

(ert-deftest compile-single-dir ()
  (let ((test-dir (expand-file-name "snippets/emacs-lisp-mode"
                                    (file-name-directory
                                     (or (buffer-file-name) (load-file))))))
    (yas/compile-directory test-dir)
    (should (file-readable-p (expand-file-name ".yas-compiled-snippets.el" test-dir)))))

For me I can't load/reload a snippet directory without any sub-directories without the patch. I can't compile that directory either.

[joaotavora]

Hmmm, I see, but you are mistaken. yas/compile-directory is meant for top-level snippet dirs, the ones that are elements of yas/snippet-dirs. In your example, it's meant for snippets/ not for snippets/emacs-lisp-mode. See it's doc, it's a bit like yas/load-directory

Good point about .yas-setup.el, I think I'm going to stop including .yas-setup.el in .yas-compiled-snippets.el.

Regarding c4b310a, I'll create another branch called visit-compiled-snippets soon. We can keep the discussion here, I'm going to close all other pull-requests/issues.

[mattfidler]

João,

I would prefer to keep .yas-setup.el in .yas-compiled-snippets.el. As you said previously, it allows a single file to represent a snippet collection. Additionally, by gzipping the file, you fulfill enhancement #119. You could name it anything and load the snippet collection easily since emacs loads both .el.gz and .elc.gz files. If you believe that .yas-setup.el is the most important file, then just include it in the md5 sum.

As far as yas/compile-directory, I understand that my use case was different from what you intended. Therefore, it was a feature not a bug :).

I would request, then, that both yas/compile-directory and yas/load-directory to load individual snippet directories as well as top-level snippet directories. It expands it's functionality, and keeps the previous functonality. Often, I prefer just to load a single snippet directory instead of a full top-level directory. Do I need to add another issue for this functionality?

Additionally, this allows a single file to be created for a snippet collection. Therefore, a snippet bundle can easily be created for major-modes that want to include specific snippets generated from yasnippet.

[mattfidler]

As far as issue #119 is concerned, by adding the ability to edit snippets that aren't present in the filesystem, and then possibly recompiling (and maybe even deleting the files afterward to save on space), I believe the enhancement is 90% fulfilled. It wouldn't be that hard to add commands to compile a zip-bundle into a el.gz bundle.

[joaotavora]

Matthew,

I see your points. I'll get back to this issues over the next week, I have real life to attend to, doing #119 would be great I agree.

[mattfidler]

I will be out next week too :)

[mattfidler]

Without 6ba04fd and bea8b79, I cannot run the current auto-compilation branch. Instead of compiling and placing the compilation in a file, it compiles and places the compilation in the current buffer. You could also add (with-temp-file ) to write the compilation to the appropriate file.

[joaotavora]

Funny, I did spot and fix a bug (see 7c094cd), but nothing like what you say. Are you sure you're using the very latest? I saw that behaviour once in some tests, but I think I fixed it. Can you give a very simple recipe for reproduction?

[mattfidler]

Sorry. My mistake. I didn't pull the latest.