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

Memoize #17

Closed
wants to merge 16 commits into from
Closed

Memoize #17

wants to merge 16 commits into from

Conversation

darkleaf
Copy link
Owner

No description provided.

@darkleaf darkleaf force-pushed the master branch 2 times, most recently from f1e8c12 to 71983f5 Compare October 23, 2024 13:06
@darkleaf darkleaf force-pushed the memoize branch 2 times, most recently from 4d3099f to f5ebb03 Compare October 23, 2024 14:25
@@ -0,0 +1,53 @@
(ns darkleaf.di.tutorial.z-memoize-test ;;todo: name
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

этот тест нужно переработать, чтобы это была как документация

src/darkleaf/di/core.clj Outdated Show resolved Hide resolved
(di/use-cache cache))
_ (di/start [::y `a]
{::y :y}
(di/log (callbacks :third))
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

тут порядок важен

{::y :y} по идее должен быть после use-cache.
log должен быть самым последним.

НО если лог последний, то мы увидим, что компоненты из main по 3 раза останавливаются.

KGOH
KGOH previously approved these changes Oct 24, 2024
src/darkleaf/di/core.clj Outdated Show resolved Hide resolved
@darkleaf darkleaf force-pushed the master branch 2 times, most recently from f73a37f to 073471c Compare October 28, 2024 18:45
@darkleaf darkleaf force-pushed the memoize branch 3 times, most recently from 297c0f9 to cfcc33c Compare October 29, 2024 14:49
(set! *warn-on-reflection* true)

(t/deftest cache-stop-test
(let [cache (atom {})
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(let [cache (->cache)] ... ???

(downstream-registry key)))]
(fn [key]

;; throw on empty cache?
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!

(let [cached-registry (:registry @cache)
registry (fn [key]
;; тут бы тест написать на порядок
;; в gmonit хороший пример с reitit и update-key conj routes
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!

"
[cache]
(fn [downstream-registry]
(let [cached-registry (:registry @cache)
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

может быть выше делать один глобальный deref?

@@ -870,3 +872,66 @@
middlewares
(inspect-middleware))]
@components))

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(def cache (di/cache))
(di/start (:collect cache))
(di/start (:use cache))
(def cache (di/cache)) ;; or (def cache (atom {}))
(di/start (di/collect-cache cache))
(di/start (di/use-cache cache))

di/cache -> di/->cache ??

(fn [registry]
(swap! cache assoc :registry (fn [key]
(case key
::implicit-root nil
Copy link
Owner Author

@darkleaf darkleaf Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

::implicit-root в системах, использующих кэш, свой, со своими зависимостями.
Его нельзя кэшировать.

Есть еще вариант в start положить реестр с рутом самым последним, но тогда отваливается add-side-depencency.

           middlewares (concat [with-env
-                               with-ns
-                               root-registry]
-                              middlewares)
+                               with-ns]
+                              middlewares
+                              [root-registry])
           registry    (apply-middleware nil-registry middlewares)

https://github.com/darkleaf/di/pull/35/files

Хз что лучше.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

от add-side-dependency одни проблемы. Может быть переделать ее на
(add-side-dependency key dep)
и по умолчанию (add-side-dependency ::implicit-root dep)?

или сделать add-side-dependency2, а первую задепрекейтить?

@darkleaf
Copy link
Owner Author

darkleaf commented Nov 8, 2024

#36

@darkleaf darkleaf closed this Nov 8, 2024
@darkleaf darkleaf deleted the memoize branch November 8, 2024 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants