Skip to content

Memoize3#45

Closed
darkleaf wants to merge 1 commit intomasterfrom
memoize3
Closed

Memoize3#45
darkleaf wants to merge 1 commit intomasterfrom
memoize3

Conversation

@darkleaf
Copy link
Owner

@darkleaf darkleaf commented Mar 29, 2025

closes #36

@darkleaf darkleaf changed the base branch from master to memoize2 March 29, 2025 18:43
@darkleaf darkleaf changed the base branch from memoize2 to master March 30, 2025 16:20
@darkleaf darkleaf force-pushed the memoize3 branch 3 times, most recently from b548443 to 3289bae Compare April 2, 2025 07:43
@@ -0,0 +1,54 @@
(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.

Тут предполагается написать тест-документацию.

А тест выше - технический.

Можно по идее технический тест сюда положить.

Copy link
Collaborator

Choose a reason for hiding this comment

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

лучше наш реальный юзкейс сюда

p/description
;; хз надо ли так
;; у нас тут нет способа понять уже закешировано или только сейчас
#_(assoc ::memoized true))))))))))
Copy link
Owner Author

@darkleaf darkleaf Apr 3, 2025

Choose a reason for hiding this comment

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

will-be-memoized


(defn ->memoize
"Returns a statefull middleware that memoizes all registry build accesses.
Must be the last in a middleware chain.
Copy link
Collaborator

Choose a reason for hiding this comment

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

memoizes all components before itself and doesn't affect the ones after it

Copy link
Owner Author

Choose a reason for hiding this comment

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

Не согласуется с предыдущим предложением.

надо подумать.

@darkleaf darkleaf force-pushed the memoize3 branch 3 times, most recently from fe5b8d1 to 7635db8 Compare April 3, 2025 15:38
(apply [_ previous-registry]
(when-not (-> previous-registry meta ::idx zero?)
(throw (ex-info "memoize should be first" {})))
(let [registry (apply-middlewares previous-registry middlewares 0)]
Copy link
Owner Author

Choose a reason for hiding this comment

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

это будет вычисляться на каждый di/start, наверное это не является проблемой?

Function
(apply [_ previous-registry]
(when-not (-> previous-registry meta ::idx zero?)
(throw (ex-info "memoize should be first" {})))
Copy link
Owner Author

Choose a reason for hiding this comment

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

todo: name

Comment on lines 115 to 103
(-> (di/start `a mem)
(di/stop))
(t/is (= [[:start ::param]
[:start `a]
[:start ::di/implicit-root]]
@log))))
Copy link
Owner Author

Choose a reason for hiding this comment

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

может быть этот кусочек отдельным тестом?
тут проверяется, что .close на мапе вызывается

(apply [_ previous-registry]
(when-not (-> previous-registry meta ::idx zero?)
(throw (ex-info "memoize should be first" {})))
(let [registry (apply-middlewares previous-registry middlewares 0)]
Copy link
Owner Author

@darkleaf darkleaf Apr 23, 2025

Choose a reason for hiding this comment

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

  1. это должно происходить уровнем выше. чтобы этого добиться нужно выпилить/переделать implicit-root. чтобы implicit-root не было в реестре, и чтобы цепочка реестров не зависила от ключа.
  2. это позволит делать проверку реестров по identical?
  3. нужно кэшировать фабрики, их инстанцирование занимает много времени
  4. функа в computeIfAbsent должна использовать ключ, а не замыкания

@darkleaf darkleaf force-pushed the memoize3 branch 3 times, most recently from 5876981 to e5f602f Compare April 25, 2025 04:42
(when-not (identical? previous-registry initial-registry)
(throw (ex-info "memoize should be first" {})))
(fn [key]
(let [factory #_(registry key) (.computeIfAbsent factories key registry)]
Copy link
Owner Author

Choose a reason for hiding this comment

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

!

Comment on lines +236 to +237
(concat (seq (p/dependencies factory))
(seq {::side-dependency :optional})))
Copy link
Owner Author

Choose a reason for hiding this comment

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

into

will-be-memoized

refactor test

fix

clear

idea

test

fix

.clear test

refactoring

fix

test

add profiler

idea 2

next

save

fix
@darkleaf
Copy link
Owner Author

closed by #54

@darkleaf darkleaf closed this Apr 25, 2025
@darkleaf darkleaf deleted the memoize3 branch April 25, 2025 14:20
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