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
Improve resource sync performance #8674
Conversation
(defn- combine-snapshots [snapshots] | ||
(reduce | ||
(fn [result snapshot] | ||
(if-let [collisions (seq (clojure.set/intersection (resource-paths result) (resource-paths snapshot)))] | ||
(if-let [collisions (not-empty (map-intersection (:status-map result) (:status-map snapshot)))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very important addition! On my machine, this change brought down resource sync time from 4.5 to 2 seconds.
(make-debugger-snapshot workspace) | ||
(make-library-snapshots new-library-snapshot-cache lib-states))) | ||
:snapshot-cache new-library-snapshot-cache})) | ||
(resource/with-defignore-pred project-directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very important addition (especially for Windows) — we no longer check canonical path and mtime of .defignore
for every file.
:status-map (into {} (map (fn [resource] | ||
(let [path (resource/proj-path resource) | ||
version (str zip-file-version ":" (crc path))] | ||
[path {:version version :source :library :library uri-string}])) | ||
flat-resources))})) | ||
:status-map (into {} | ||
(map (fn [resource] | ||
(let [path (resource/proj-path resource) | ||
version (str zip-file-version ":" (crc path))] | ||
[path {:version version :source :library :library uri-string}]))) | ||
flat-resources)})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a minor change that was easy to do: use transducers instead of lazy seqs.
(into {} (map (juxt resource/proj-path identity) (resource/resource-list-seq (:resources snapshot))))) | ||
(into {} (map (juxt resource/proj-path identity)) (resource/resource-list-seq (:resources snapshot)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a minor change that was easy to do: use transducers instead of lazy seqs.
(let [prefixes (into | ||
#{} | ||
(filter #(string/starts-with? % "/")) | ||
(string/split-lines (slurp defignore-file)))] | ||
(let [prefixes (into [] | ||
(comp | ||
(filter #(string/starts-with? % "/")) | ||
(distinct)) | ||
(string/split-lines (slurp defignore-file)))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a minor change that was easy to do: use distinct vec instead of set for faster ignored path check. Very minor effect on performance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good, apart from you fixing a typo now makes a comment inaccurate due to the field mentioned actually being misspelled in Bob. 😞
I'll let you decide how you want to deal with it.
editor/src/clj/editor/resource.clj
Outdated
;; If you change something here, plese change it there as well | ||
;; Search for excluedFilesAndFoldersEntries. | ||
;; If you change something here, please change it there as well | ||
;; Search for excludedFilesAndFoldersEntries. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, the name of the field is misspelled in Bob
. 😞
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh no
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was me 😳
* Improve resource sync performance Fixes #8595 * Bring back the misspelling
Fixes #8595