search

kostafey / ejc-sql

Emacs SQL client uses Clojure JDBC.
278 stars 29 forks source link

Speed up auto-completion when using company #187

Open dvzubarev opened 11 months ago

dvzubarev commented 11 months ago

This is follow-up of https://github.com/kostafey/ejc-sql/issues/141 Completions on words feels very slow. You can benchmark it with following code:

(require 'benchmark)
(benchmark-elapse (ejc-company-candidates "se"))

It gives results in range of 0.3 - 0.5 seconds. Completion time reduces to 0.0011 after this change.

If the point is on dot (name.| or name.var|), then synchronously collect candidates. Otherwise use cached data. When cache is empty (first time invocation), it returns only common SQL words and schedules the cache update. Delayed cache update is done via run-with-idle-timer. Also cache is updated when it is older than ejc-company-cache-update-ivl-secs (60 by default).

Also these changes helps in case of fatal server error (like https://github.com/kostafey/ejc-sql/issues/155). Company will complete common sql keywords and report the error, instead of just reporting the error.

stardiviner commented 4 months ago

Sorry for late respond. I checked this PR, but forgot to reply, until I reviewed my Org tasks, picked up this issue again.

If the point is on dot (name.| or name.var|), then synchronously collect candidates. Otherwise use cached data. When cache is empty (first time invocation), it returns only common SQL words and schedules the cache update. Delayed cache update is done via run-with-idle-timer. Also cache is updated when it is older than ejc-company-cache-update-ivl-secs (60 by default).

This design is sophisticated and fine for me. I think it's good to be merged.

stardiviner commented 2 months ago

Hi, @kostafey , Will this commit be merged into the master branch?

kostafey commented 2 months ago

Hi, @kostafey , Will this commit be merged into the master branch?

Hello. I was blocked by GitHub (details) for some period of time. Now I'm back. So I'll return all repos links from GitLab to GitHub soon.

Regarding this issue. I think we can merge this changes if user will have an option to maintain the previous behavior (e.g. via custom variable).

stardiviner commented 2 months ago

Regarding this issue. I think we can merge this changes if user will have an option to maintain the previous behavior (e.g. via custom variable).

I agree. BTW, I move all github repos to other OOS hosting websites before. I know big company is evil.