You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by davisp <gi...@git.apache.org> on 2016/12/06 19:39:49 UTC
[GitHub] couchdb-couch pull request #217: Remove use of filename:rootname/1
GitHub user davisp opened a pull request:
https://github.com/apache/couchdb-couch/pull/217
Remove use of filename:rootname/1
It turns out that filename:rootname/1 is fairly expensive. Given that we
call it millions of times when doing database name validation it adds up
significantly. This is a fairly easy optimization to just use binary
matching instead.
COUCHDB-3251
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cloudant/couchdb-couch 3251-remove-filename-rootname
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-couch/pull/217.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #217
----
commit e8a97f52b0cf0d0408d5a4ea625644684edaa6f5
Author: Paul J. Davis <pa...@gmail.com>
Date: 2016-12-06T19:37:45Z
Remove use of filename:rootname/1
It turns out that filename:rootname/1 is fairly expensive. Given that we
call it millions of times when doing database name validation it adds up
significantly. This is a fairly easy optimization to just use binary
matching instead.
COUCHDB-3251
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch issue #217: Remove use of filename:rootname/1
Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on the issue:
https://github.com/apache/couchdb-couch/pull/217
Oh whoops. I forgot to run the full suite. Just ran couch tests. Will check the full run and also add a benchmark as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch issue #217: Remove use of filename:rootname/1
Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on the issue:
https://github.com/apache/couchdb-couch/pull/217
Also for posterity, someone else was looking at this and made this comment:
>The reason for the high cost of filename:rootname is that it is recursive, and the number of loops
it runs is proportional to the string length that is passed in as parameter. This is why we see the number of calls to it is very high.
I thought that was an interesting note so I've copied it here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch issue #217: Remove use of filename:rootname/1
Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on the issue:
https://github.com/apache/couchdb-couch/pull/217
Here's a micro benchmark that shows a significant speedup for the new version. Between 1,000% and 6,000% depending on whether it actually has to remove the suffix.
```erlang
-module(benchmark).
-export([
go/0,
drop_dot_couch_ext/1,
maybe_remove_extension/1
]).
-define(TEST_1, <<"shards/00000000-1fffffff/example/c28fb579-0a4c-4421-8d44-65a7650b6da0.1475265285">>).
-define(TEST_2, <<"shards/00000000-1fffffff/example/c28fb579-0a4c-4421-8d44-65a7650b6da0.1475265285.couch">>).
go() ->
Tests = [
fun() -> run_drop(?TEST_1, 1000) end,
fun() -> run_drop(?TEST_2, 1000) end,
fun() -> run_maybe(?TEST_1, 1000) end,
fun() -> run_maybe(?TEST_2, 1000) end
],
lists:foreach(fun(T) ->
R = timer:tc(T),
io:format("~p~n", [R])
end, Tests).
run_drop(_Test, 0) ->
ok;
run_drop(Test, Count) ->
?TEST_1 = drop_dot_couch_ext(Test),
run_drop(Test, Count - 1).
run_maybe(_Test, 0) ->
ok;
run_maybe(Test, Count) ->
?TEST_1 = maybe_remove_extension(Test),
run_maybe(Test, Count - 1).
drop_dot_couch_ext(DbName) when is_binary(DbName) ->
PrefixLen = size(DbName) - 6,
case DbName of
<<Prefix:PrefixLen/binary, ".couch">> ->
Prefix;
Else ->
Else
end;
drop_dot_couch_ext(DbName) when is_list(DbName) ->
binary_to_list(drop_dot_couch_ext(iolist_to_binary(DbName))).
maybe_remove_extension(DbName) ->
case filename:extension(DbName) of
<<".couch">> ->
filename:rootname(DbName);
_ ->
DbName
end.
```
```
Eshell V6.4.1.5 (abort with ^G)
1> benchmark:go().
{141,ok}
{124,ok}
{1285,ok}
{5673,ok}
ok
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch issue #217: Remove use of filename:rootname/1
Posted by nickva <gi...@git.apache.org>.
Github user nickva commented on the issue:
https://github.com/apache/couchdb-couch/pull/217
+1
```
(node1@127.0.0.1)1> couch_util:drop_dot_couch_ext("").
[]
(node1@127.0.0.1)2> couch_util:drop_dot_couch_ext("abc").
"abc"
(node1@127.0.0.1)3> couch_util:drop_dot_couch_ext("a.couch$").
"a.couch$"
(node1@127.0.0.1)4> couch_util:drop_dot_couch_ext("a.couch").
"a"
(node1@127.0.0.1)5> couch_util:drop_dot_couch_ext(<<".couch">>).
<<>>
(node1@127.0.0.1)6> couch_util:drop_dot_couch_ext(<<"..couch">>).
<<".">>
(node1@127.0.0.1)7> couch_util:drop_dot_couch_ext(<<".couch">>).
<<>>
(node1@127.0.0.1)9> couch_util:drop_dot_couch_ext(<<"shards/00000000-1fffffff/a/b.1475265285.couch">>).
<<"shards/00000000-1fffffff/a/b.1475265285">>
```
Tests are failings, but that's a different error (mrview tests I think)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch issue #217: Remove use of filename:rootname/1
Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on the issue:
https://github.com/apache/couchdb-couch/pull/217
And yeah, the couchdb_mrview_tests module failed with a bad response code which didn't happen locally (those are actually in src/couch/test which I ran). Most odd.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch issue #217: Remove use of filename:rootname/1
Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on the issue:
https://github.com/apache/couchdb-couch/pull/217
Oh heh, the travis CI check doesn't even include this commit when it runs the tests so I'm just gonna go ahead and ignore that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request #217: Remove use of filename:rootname/1
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/couchdb-couch/pull/217
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---