Decide whether to replace custom JSON parsing in maktaba#plugin#AddonInfo()
malcolmr opened this issue · 2 comments
Currently, maktaba#plugin#AddonInfo() has a hand-crafted JSON parser that calls eval() after verifying (hopefully) that the JSON is safe. This is fast, but perhaps undesirable.
In #152, we've just added a fast Python-based implementation of maktaba#json#Parse(), so it seemed reasonable to see what impact using that instead would have on plugin loading.
I tried loading the standard sets of plugins we have at work (which are grouped into packages of different sizes: core, default, and large, each of which is a subset of the next). Loading from local disk, the measurements I see are:
| loading with | core | default | large |
|---|---|---|---|
existing s:EvalJSON() hack |
180ms | 381ms | 450ms |
maktaba#json#Parse() (Python) |
192ms (+12ms) | 398ms (+17ms) | 472ms (+22ms) |
maktaba#json#Parse() (Vimscript) |
193ms (+13ms) | 467ms (+86ms) | 498ms (+48ms) |
(I can't quite explain the change in absolute difference for the Vimscript implementation, since loading the large package should take at least the additional time taken to load the default package. Nevertheless, that's what the figures seem to suggest.)
There are therefore no cases (on this hardware) in which using maktaba#json#Parse() instead of our existing regexp/eval hack is faster in practice for plugin loading, though the former is certainly safer (and less code).
However, the cost seems small (≲20ms in most cases), and we get to remove some code (which calls eval on untrusted data), so it's possibly worth doing.
Thoughts?
Tradeoff seems totally reasonable to me. The next big win will be getting vim to implement native jsonparse and jsondump (https://code.google.com/p/vim/issues/detail?id=383) and preferring that implementation.
Those are total startup times, right? Are you able to isolate how much of the total time JSON parsing accounts for?
Time spent in maktaba#plugin#AddonInfo():
| loading with | average time per plugin |
|---|---|
existing s:EvalJSON() hack |
275μs plus ~40μs one-time cost |
maktaba#json#Parse() (Python) |
395μs plus 12500μs one-time cost |
maktaba#json#Parse() (Vimscript) |
2600‒3025μs plus 1000μs(?) one-time cost |
The Vimscript implementation is also affected by the size of the addon-info.json file, and the extra plugins in our 'large' package tend to have more dependencies, hence the range in averages above.
The core, default, and large packages I'm using have 6, 31, and 39 plugins respectively, and the total plugin time spend in maktaba#plugin#AddonInfo() is (roughly) proportional to the average per plugin:
| loading with | core | default | large |
|---|---|---|---|
existing s:EvalJSON() hack |
1.6ms | 8.5ms | 11ms |
maktaba#json#Parse() (Python) |
15ms | 25ms | 28ms |
maktaba#json#Parse() (Vimscript) |
17ms | 93ms | 119ms |
(I'm aware that those don't entirely match the figures above; they should be approximately correct, though. The figures is the first comment are more reliable, since I was able to average over multiple runs and discard outliers.)