Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vital モジュールのロードを爆速にする & ロード方法を考える #415

Merged
merged 83 commits into from Apr 29, 2016

Conversation

@haya14busa
Copy link
Member

haya14busa commented Mar 24, 2016

概要

使い方

  1. :Vitalize . でアップデートしたり新たに :Vitalize
  2. vital#of({plugin-name}) の変わりに vital#{plugin-name}#of() を使う

これだけ!
ただ,vital#of({plugin-name})でも速くなるようにしています. except vital#of('vital')

現時点で既に対応しているプラグイン

ベンチマーク

TODO

注意点

  • vital#of('vital')#137 の外部モジュールを同梱する機能 (個人的にself moduleとよんでいる) は速くなりません.

vital#of をどうするか?

  1. vital#of({plugin-name}) はそのままで,速くしたい場合は vital#{plugin-name}#of()を導入
  2. vital#of({plugin-name}) のインターフェースはそのままで内部で早くする.vital#{plugin-name}#of()は導入しない
  3. vital#of({plugin-name}) のインターフェースはそのままで内部で早くする.vital#{plugin-name}#of()も導入する
  4. vital#of({plugin-name}) は組み込まず vital#{plugin-name}#of()に完全移行
    • 厳密にはvital#of('vital')相当は残す or 別の対応する機構を提供
  5. そもそもvitalのローダ作る必要なくね? vital#{plugin-name}#import({module-name}) とかに移行する.

などなどがあります

あと少なくとも新しいvitalオブジェクトでは s:V.search(), s:V.load(), s:V.exists() とか別にいらないんじゃないかと思うみたいな思いもあります

疲れたのであとは随時追記するかコメントします

@haya14busa
Copy link
Member Author

haya14busa commented Mar 24, 2016

@haya14busa haya14busa force-pushed the haya14busa:revital branch from d9a34f9 to 7b7c26f Mar 24, 2016
@tyru
Copy link
Member

tyru commented Mar 24, 2016

関連 #40 #41

@rhysd
Copy link
Contributor

rhysd commented Mar 24, 2016

関連記事:

http://haya14busa.com/revital-vim-makes-vital-vim-a-lot-faster/

速くなるのは間違いないと思うので入れる方向で行きたいです.

@haya14busa
Copy link
Member Author

haya14busa commented Mar 24, 2016

冒頭コメントちょっとアップデートした.

@haya14busa
Copy link
Member Author

haya14busa commented Mar 24, 2016

このへんに記事に対しての thinca さんのコメント http://lingr.com/room/vim/archives/2016/03/23#message-23153686

@haya14busa
Copy link
Member Author

haya14busa commented Mar 24, 2016

revital で落ちてなかったのに落ちてるのはナンデや(古いvim)...まぁまた対応します

@thinca
Copy link
Member

thinca commented Mar 24, 2016

インストールされる各モジュールを書き換えることは、モジュールの製作者が予期しない副作用が発生する可能性があるので消極的です。(絶対ダメとは言っていない)

@haya14busa
Copy link
Member Author

haya14busa commented Mar 24, 2016

はい,まぁそれがわかってるから最初は別リポジトリにしたんですが流れ的に行けるかなと(流れ力)

@haya14busa
Copy link
Member Author

haya14busa commented Mar 24, 2016

s:___vital_function___ 以外はスクリプト変数も関数も汚してないですが,まぁ逆に言うとひとつは汚してはいる

@haya14busa
Copy link
Member Author

haya14busa commented Mar 24, 2016

thincaさんが前に言っていたようなsidのみ返す関数にしても同様にスクリプト変数1つ,またはスクリプト関数1つ定義しなきゃだめですね

@thinca
Copy link
Member

thinca commented Mar 24, 2016

あーあと使用するローダーを :Vitalize のオプションで指定できるようにして、新版は明示的に指定した時のみ使うようにすると良いかも。移行期間的な。gcc の -std=c++14 的な感じで(ちょっと違うけど)。
ただめっちゃ面倒そうなので、どうしても面倒ならそれもやむなし。

@haya14busa
Copy link
Member Author

haya14busa commented Mar 24, 2016

選択はたしかに有りな気はしますね

@haya14busa
Copy link
Member Author

haya14busa commented Mar 24, 2016

s:___vital_function___ 以外はスクリプト変数も関数も汚してないですが,まぁ逆に言うとひとつは汚してはいる

これ 7.3.1170 以降は消せるので,:Vitalize 時にプラグインが7.3.1170以前に対応するかどうか決めれるみたいなのはやるといいかも

@haya14busa
Copy link
Member Author

haya14busa commented Mar 24, 2016

これ 7.3.1170 以降は消せるので,:Vitalize 時にプラグインが7.3.1170以前に対応するかどうか決めれるみたいなのはやるといいかも

冷静に考えると :Vitalizer で指定とかなしで対応できるな

@haya14busa
Copy link
Member Author

haya14busa commented Mar 25, 2016

execute 使うけどスクリプト変数もスクリプト関数も残さないようにした.便利.
テストの落ち方が謎でつらい

@haya14busa haya14busa force-pushed the haya14busa:revital branch from 25835ee to 4af4dbd Mar 25, 2016
@ujihisa
Copy link
Member

ujihisa commented Mar 25, 2016

APIについて (実装は無視):

これまでvital#of("aaa")みたいにof()という関数を提供してたのはconstructor fuctoryとしてほとんど無名にしたかったための命名でした。今回vital#aaa#of()みたいにユーザプラギン名が文字列でない形ででてくるなら、実装が可能ならofすらとってしまってvital#aaa()みたいにするとより便利さ。(実装上うまく安全に混ぜることができないならスルー力で)

@haya14busa haya14busa force-pushed the haya14busa:revital branch from d84651d to 2fcffc6 Mar 25, 2016
@rhysd
Copy link
Contributor

rhysd commented Mar 26, 2016

vital#aaa だとファイルをプラギンごとに分けられないと思います.ただ,確かに vital#plugin#of は微妙ですね…

@haya14busa
Copy link
Member Author

haya14busa commented Mar 26, 2016

vital#pluginname() は無理ですね.

ただ,確かに vital#plugin#of は微妙ですね…

vital#pluginname() は無理となったら結局 vital#pluginname#{無名にちかいconstruct関数名} にしなきゃだめで vital#of() の議論と同じになるのではないでしょうか?

インターフェース追加して vital#pluginname#import() とかはありかなとかは考えてますが, vital#of()相当は残したほうが移行もスムーズにできるし便利そう.

@haya14busa haya14busa force-pushed the haya14busa:revital branch 3 times, most recently from 09e87d4 to 379dacb Mar 26, 2016
@haya14busa
Copy link
Member Author

haya14busa commented Apr 25, 2016

はい.あってます.appveyor の vim7.3 build でヨクワカラナイエラーが出ることを本当は修正したい...原因究明したい...という以外は何もやることは残ってないはずです.

ただテスト通っているのは別のテストをスキップしてたので対象の vital.vimspec をスキップするように修正しました.

@haya14busa
Copy link
Member Author

haya14busa commented Apr 25, 2016

厳密にこのPRのheadではなくなってる気もするけど, incsearch.vimやeasymotoinでは前からこのPRをmasterで試していて,今の所おかしくなったというissueはないです.

@Shougo
Copy link
Member

Shougo commented Apr 25, 2016

unite.vim, neocomplete 等で使用していますが、特に問題はないです。

@ujihisa
Copy link
Member

ujihisa commented Apr 25, 2016

マージ & ブログ記事たのしみです!

@tyru
Copy link
Member

tyru commented Apr 26, 2016

Windows on Vim 7.3 で動かない件は Changes に書いておく必要はありそうですね。
ただもう Vim 7.3 を使う人も少なくなってきてるような気はするので、思った程問題ないような気もします。

  • Vim プラグイン開発者に求めること:Windows on Vim 7.3 では現状動かない事を理解した上で vital を使ってもらう
  • このPRで行うこと (Windows on Vim 7.3 で動かない件に対して)
    • known issue としてdocに書く (tyru が別PRでやる)
    • doc の vital#of('vital')vital#{plugin-name}#of('vital') に変更 (tyru が別PRでやる)
    • Issue 作成 🎫 #426
    • マージ 🎉

自分はこの件に関してこれで十分と感じてるんですが、皆さんはどうでしょうか?

@haya14busa
Copy link
Member Author

haya14busa commented Apr 26, 2016

Windows on Vim 7.3 で動かない件は Changes に書いておく必要はありそうですね。
ただもう Vim 7.3 を使う人も少なくなってきてるような気はするので、思った程問題ないような気もします。

実はそれよりももっと条件は狭まっていて,

  1. すくなくとも起こるとしても :Vitalize時関連.(またはテスト)
  2. 他のテスト(System.Process.Vimproc)をskipしたりすると通ることから,普通に:Vitalizeする分にはVim 7.3 on Windowsだろうが動く
  3. このPRでバグが入ったわけではなく,そもそもmasterのvitalでもテストのみ移植すると再現する

という感じです.

なので Changes に書くようなことではないかな.
丁寧にやるなら issue 作成プラスknown issue としてdocに書くくらいでしょうか.

@haya14busa
Copy link
Member Author

haya14busa commented Apr 26, 2016

なので

Vim プラグイン開発者に求めること:Windows on Vim 7.3 では現状動かない事を理解した上で vital を使ってもらう

これは言い過ぎで,Windows on Vim 7.3 で:Vitalizeする際に特定の条件で失敗する可能性がある.というだけなはずです.
プラグインを使うユーザに関しては関係ないはず.

@tyru
Copy link
Member

tyru commented Apr 26, 2016

あー vitalizer の話だったんですね。それじゃ Changes に書かなくても良さそうですね。

haya14busa added a commit to haya14busa/vim-operator-flashy that referenced this pull request Apr 26, 2016
function! s:install_module_files(module_files, plugin_name, to) abort
" List and check the installing module_files.
let install_files = []
" for f in files + s:LOADER_FILES

This comment has been minimized.

Copy link
@haya14busa

haya14busa Apr 27, 2016

Author Member

コメントアウトのこってたので後で消します

@tyru
Copy link
Member

tyru commented Apr 27, 2016

あとこのタスクもありました (上のタスクリスト更新)。

さすがに既存のhelpのvital#of('vital')修正して回る作業をここでするとdiffが死ぬので別で

上ではドキュメントの修正もタスクとして上げちゃいましたが、こういったドキュメントの修正であれば自分がGWに別PRでやるので、 @haya14busa さんがもう修正の予定がないならマージしても良いと自分は思います。 :shipit:

@thinca
Copy link
Member

thinca commented Apr 29, 2016

@haya14busa マージしちゃっても良いかな?

@haya14busa
Copy link
Member Author

haya14busa commented Apr 29, 2016

👍👍👍👍👍

@haya14busa
Copy link
Member Author

haya14busa commented Apr 29, 2016

vital#of() -> vital#vital#of のドキュメント修正は単にdiff増やしたくなかっただけで grep からの qfreplaceで多分しゅっと直せるけど,やっていただけるなら任せちゃおうかな?

あと,test で呼ばれてる vital#of() も vital#vital#of() にかえれそう

@thinca thinca merged commit 30b51b8 into vim-jp:master Apr 29, 2016
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@haya14busa
Copy link
Member Author

haya14busa commented Apr 29, 2016

マッジありがとうございますっ

@haya14busa haya14busa deleted the haya14busa:revital branch Apr 29, 2016
tyru added a commit to tyru/open-browser.vim that referenced this pull request May 1, 2016
tungel added a commit to tungel/.vim that referenced this pull request May 12, 2016
5c6f3cd update vital
a6b3c10 Merge pull request #285 from easymotion/rerevitalize
02884f8 Try vim-jp/vital.vim#415
2dad1b1 Merge pull request #282 from easymotion/revitalizer
07e3665 Update vital
cdc77ad introduce revitalizer for speed!!!

git-subtree-dir: bundle/vim-easymotion
git-subtree-split: 5c6f3cd9a713491e6b32752a05c45198aa91540a
NonWonderDog added a commit to NonWonderDog/.vim that referenced this pull request May 19, 2016
e2bfabf Merge pull request #87 from tyru/upgrade-vital
5f8d5e7 Upgrade to new vital (vim-jp/vital.vim#415)
5170a17 Merge pull request #85 from tyru/fix/cant-open-filepath
ab2a06a Fix: can't open filepath
6cb4bd3 Update *openbrowser-thanks*
9aefebf Merge pull request #83 from tyru/vim-jp-redirects
85338d9 [AppVeyor] Use vim-jp.org/redirects
505e21c Use suite.after_each(), don't :catch
64f4cad Doc: Update g:openbrowser_no_default_menus description
2db88f0 Add menu on GUIEnter
3fa43a6 Use <Nop> for menu-separator
8a11108 Don't map Select-mode
62c3d27 Cosmetic fix
db04dcc Use :menutrans
41f64e0 g:restart_no_default_menus: Respect &guioptions =~# 'M'
8b342c9 ':OpenBrowser' won't open 'http://203.0.113.11' (Fix #82)
6a8eb25 Add appveyor.yml
c056913 Update README.md
910cd8b Fix test for AppVeyor
cfc0743 Create README.md
881195a Update .travis.yml
e2e57f4 Add tests for MS Windows
b05377d oops s/themis#suite/themis#helper/
c28b2da Add tests with some refactorings
2456f74 Write basic test (only for Linux)
4deae48 Update help
fde2c45 (Fix #81) ':OpenBrowserSmartSearch tyru/open-browser.vim' fails
15e9b3f Make open-browser.vim testable (#1)
fcbdcc9 Merge pull request #80 from anekos/fix/allow-the-url-without-dots
8123820 Allow the url without dots
a1894bc Add .travis.yml
0476982 Fix: infinite loop bug
122f365 Fix: ':OpenBrowserSmartSearch foo bar' throws an error
e4d528a Fix warnings/errors found by vimlint
2ec2875 Improve URL detection
b0326dc Fix <Plug> keymappings tags
61169d9 Small refactoring
f2c50a3 Update vital
616949c Refactoring
39b0190 Fix s:Web.URI._eat_em(): empty string is not 'no match' (#72)
658993b More tolerant URI parsing (#72, #73, #74)
6072647 Fix comments
5dc8916 Merge pull request #74 from rhysd/patch-1
d9983a1 :OpenBrowser should open file with browser by default
e175358 Fix: #71 change broke #70
ce1a1dc More torrelant behavior when there are no words on cursor (#71)
10ee02c Fix deprecation warnings (#70)
f8ca30b Open multiple URLs when selecting in visual-mode (#70)

git-subtree-dir: bundle/open-browser
git-subtree-split: e2bfabfc758351ee3087d4c697d1953660d943ee
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants
You can’t perform that action at this time.