You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by "nickva (via GitHub)" <gi...@apache.org> on 2024/02/16 21:05:39 UTC
[PR] Replace khash with maps in ddoc_cache_lru [couchdb]
nickva opened a new pull request, #4987:
URL: https://github.com/apache/couchdb/pull/4987
This is a companion PR to the previous one for couch_event_server [1].
Implementation notes:
* Some functionality was moved to helper functions (do_refresh, get_entries)
* The main concern was to make sure to return the updated map object, when before we just returned `ok`.
* ddoc_cache_lru already had an almost 100% test coverage so opted to rely those tests.
* Performance with maps seems to at least as good or better than with the khash nif [2].
[1] https://github.com/apache/couchdb/pull/4977
[2] https://gist.github.com/nickva/06f1511b7f9d0bbc2d9a0dfc2d36779e
**Performance benchmark**
(MacOS, Intel, Erlang/OTP 24)
With maps:
```
> ddoc_cache_bench:go().
Dbs DocIds Revs Open(Dt/Key) Close(Dt/Key)
100 100 5 202.5 11.1
1000 5 5 192.1 21.9
10000 1 1 176.8 65.6
1 10000 1 180.3 30.7
1 1 10000 185.9 32.1
GC Runs: 4733375
```
With the khash NIF:
```
> ddoc_cache_bench:go().
Dbs DocIds Revs Open(Dt/Key) Close(Dt/Key)
100 100 5 202.2 12.6
1000 5 5 193.2 26.0
10000 1 1 183.1 75.0
1 10000 1 182.8 40.0
1 1 10000 182.8 38.5
GC Runs: 4799318
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace khash with maps in ddoc_cache_lru [couchdb]
Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva commented on PR #4987:
URL: https://github.com/apache/couchdb/pull/4987#issuecomment-1949359822
@davisp I sorry. It was a good data hashing data structure long before maps existed!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace khash with maps in ddoc_cache_lru [couchdb]
Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva commented on code in PR #4987:
URL: https://github.com/apache/couchdb/pull/4987#discussion_r1492987076
##########
src/ddoc_cache/src/ddoc_cache_lru.erl:
##########
@@ -241,80 +189,87 @@ lru_start(Key, DoInsert) ->
ddoc_cache_entry:recover(Key)
end.
-trim(_, 0) ->
- full;
-trim(St, MaxSize) ->
+trim(#st{} = St, 0) ->
+ {full, St};
+trim(#st{} = St, MaxSize) ->
CurSize = ets:info(?CACHE, memory) * erlang:system_info(wordsize),
if
CurSize =< MaxSize ->
- ok;
+ {ok, St};
true ->
case ets:first(?LRU) of
{_Ts, Key, Pid} ->
- remove_entry(St, Key, Pid),
- trim(St, MaxSize);
+ St1 = remove_entry({Key, Pid}, St),
+ trim(St1, MaxSize);
'$end_of_table' ->
- full
+ {full, St}
end
end.
-remove_entry(St, Key, Pid) ->
- #st{
- pids = Pids
- } = St,
+get_entries(DbName, #{} = Dbs) ->
+ case Dbs of
+ #{DbName := DDocIds} ->
+ Fun = fun(_, Keys, Acc1) -> maps:to_list(Keys) ++ Acc1 end,
+ maps:fold(Fun, [], DDocIds);
+ _ ->
+ []
+ end.
+
+do_refresh(#{} = DDocIdsMap, [_ | _] = DDocIdList) ->
+ Fun = fun(DDocId) ->
+ case DDocIdsMap of
+ #{DDocId := Keys} ->
+ maps:foreach(fun(_, Pid) -> ddoc_cache_entry:refresh(Pid) end, Keys);
+ _ ->
+ ok
+ end
+ end,
+ lists:foreach(Fun, [no_ddocid | DDocIdList]).
+
+remove_entry({Key, Pid}, #st{pids = Pids, dbs = Dbs} = St) ->
Review Comment:
Note: `remove_entry/2` args were changed so that it can be directly used in a lists:foldl callback.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace khash with maps in ddoc_cache_lru [couchdb]
Posted by "jaydoane (via GitHub)" <gi...@apache.org>.
jaydoane commented on PR #4987:
URL: https://github.com/apache/couchdb/pull/4987#issuecomment-1951585270
Have you considered adding `ddoc_cache_bench.erl` to the repo?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace khash with maps in ddoc_cache_lru [couchdb]
Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva merged PR #4987:
URL: https://github.com/apache/couchdb/pull/4987
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace khash with maps in ddoc_cache_lru [couchdb]
Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva commented on PR #4987:
URL: https://github.com/apache/couchdb/pull/4987#issuecomment-1951703803
Hmm, that could work, but since ddoc_cache application restarts as part of test to get a clean slate, it would blow away the ddoc cache, so it might be a bit too aggressive.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org