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

fix(3517): super-simplistic fix to avoid costly AST transforms when t… #4292

Merged
merged 8 commits into from
Dec 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ rel/dev*
rel/tmpdata
share/server/main-coffee.js
share/server/main.js
share/server/main-ast-bypass.js
share/www
src/bear/
src/certifi/
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ clean:
@rm -rf src/*/.rebar
@rm -rf src/*/priv/*.so
@rm -rf src/couch/priv/{couchspawnkillable,couchjs}
@rm -rf share/server/main.js share/server/main-coffee.js
@rm -rf share/server/main.js share/server/main-ast-bypass.js share/server/main-coffee.js
@rm -rf tmp dev/data dev/lib dev/logs
@rm -rf src/mango/.venv
@rm -f src/couch/priv/couchspawnkillable
Expand Down
2 changes: 1 addition & 1 deletion Makefile.win
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ clean:
-@rmdir /s/q src\*\.rebar
-@del /f/q/s src\*.dll
-@del /f/q src\couch\priv\*.exe
-@del /f/q share\server\main.js share\server\main-coffee.js
-@del /f/q share\server\main.js share\server\main-ast-bypass.js share\server\main-coffee.js
-@rmdir /s/q tmp
-@rmdir /s/q dev\data
-@rmdir /s/q dev\lib
Expand Down
1 change: 1 addition & 0 deletions rel/reltool.config
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@
{copy, "overlay/etc"},
{copy, "../src/couch/priv/couchjs", "bin/couchjs"},
{copy, "../share/server/main.js", "share/server/main.js"},
{copy, "../share/server/main-ast-bypass.js", "share/server/main-ast-bypass.js"},
{copy, "../share/server/main-coffee.js", "share/server/main-coffee.js"},
{copy, "../src/weatherreport/weatherreport", "bin/weatherreport"},
{copy, "files/sys.config", "releases/\{\{rel_vsn\}\}/sys.config"},
Expand Down
5 changes: 2 additions & 3 deletions share/server/60/rewrite_fun.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function rewriteFunInt(fun) {

// If we have a function declaration without an Id, wrap it
// in an ExpressionStatement and change it into
// a FuntionExpression
// a FunctionExpression
if (decl.type == "FunctionDeclaration" && decl.id == null) {
decl.type = "FunctionExpression";
ast.body[idx] = {
Expand All @@ -41,7 +41,6 @@ function rewriteFunInt(fun) {
return escodegen.generate(ast);
}


function rewriteFun(funJSON) {
const fun = JSON.parse(funJSON);
return JSON.stringify(rewriteFunInt(fun));
Expand All @@ -53,4 +52,4 @@ function rewriteFuns(funsJSON) {
return rewriteFunInt(fun);
});
return JSON.stringify(results);
}
}
61 changes: 61 additions & 0 deletions share/server/60/rewrite_fun_ast_bypass.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Licensed under the Apache License, Version 2.0 (the "License"); you may not
// use this file except in compliance with the License. You may obtain a copy of
// the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
// License for the specific language governing permissions and limitations under
// the License.
//
// Based on the normalizeFunction which can be
// found here:
//
// https://github.com/dmunch/couch-chakra/blob/master/js/normalizeFunction.js

function rewriteFunInt(fun) {
// Skip lengthy AST transforms if the function passed can be
// safely wrapped in parentheses.
if (fun.startsWith('function') && fun.endsWith('}')) {
return '(' + fun + ')'
}

const ast = esprima.parse(fun);
let idx = ast.body.length - 1;
let decl = {};

// Search for the first FunctionDeclaration beginning from the end
do {
decl = ast.body[idx--];
} while (idx >= 0 && decl.type !== "FunctionDeclaration");
idx++;

// If we have a function declaration without an Id, wrap it
// in an ExpressionStatement and change it into
// a FunctionExpression
if (decl.type == "FunctionDeclaration" && decl.id == null) {
decl.type = "FunctionExpression";
ast.body[idx] = {
type: "ExpressionStatement",
expression: decl
};
}

// Generate source from the rewritten AST
return escodegen.generate(ast);
}

function rewriteFun(funJSON) {
const fun = JSON.parse(funJSON);
return JSON.stringify(rewriteFunInt(fun));
}

function rewriteFuns(funsJSON) {
let funs = JSON.parse(funsJSON);
const results = Array.from(funs, (fun) => {
return rewriteFunInt(fun);
});
return JSON.stringify(results);
}
13 changes: 13 additions & 0 deletions src/docs/src/config/query-servers.rst
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,19 @@ you can adjust the memory limitation (here, increasing to 512 MiB)::

For more info about the available options, please consult ``couchjs -h``.

.. note::
CouchDB versions 3.0.0 to 3.2.2 included a performance regression for
custom reduce functions. CouchDB 3.3.0 and later come with an experimental
fix to this issue that is included in a separate ``.js`` file.

To enable the fix, you need to define a custom ``COUCHDB_QUERY_SERVER_JAVASCRIPT``
environment variable as outlined above. The path to ``couchjs`` needs to
remain the same as you find it on your ``couchdb`` file, and the path to
``main.js`` needs to be set to ``/path/to/couchdb/share/server/main-ast-bypass.js``.

With a default installation on Linux systems, this is going to be
``COUCHDB_QUERY_SERVER_JAVASCRIPT="/opt/couchdb/bin/couchjs /opt/couchdb/share/server/main-ast-bypass.js"``
Comment on lines +78 to +79
Copy link
Contributor

Choose a reason for hiding this comment

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

Wonder if a slightly unpleasant part here could be that an edit in bin/couchdb, which is installed by rpm/deb packages, would make that that file unable to be updated automatically by subsequent package upgrades?

RPM would put updates in.rpmnew files and deb, I think?, stops and prompts the user about what to do. Prompts are easy for desktops, but can be a pain for unattended upgrades.

Copy link
Member Author

Choose a reason for hiding this comment

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

you can do this via env var, you don’t have to edit bin/couchdb — that’s just where you get the original paths that are valid for your system


.. _Mozilla SpiderMonkey: https://spidermonkey.dev/

.. seealso::
Expand Down
15 changes: 12 additions & 3 deletions support/build_js.escript
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,13 @@ main([]) ->
_ ->
[
"share/server/60/esprima.js",
"share/server/60/escodegen.js",
"share/server/60/rewrite_fun.js"
"share/server/60/escodegen.js"
]
end,

RewriteFunFile = ["share/server/60/rewrite_fun.js"],
RewriteFunWithASTBypassFile = ["share/server/60/rewrite_fun_ast_bypass.js"],

Pre = "(function () {\n",
Post = "})();\n",

Expand All @@ -85,6 +87,13 @@ main([]) ->
file:write_file(To, FinalBin)
end,

ok = Concat(ExtraFiles ++ JsFiles, "share/server/main.js"),
case SMVsn of
"1.8.5" ->
ok = Concat(ExtraFiles ++ JsFiles, "share/server/main.js"),
ok = Concat(ExtraFiles ++ JsFiles, "share/server/main-ast-bypass.js");
_ ->
ok = Concat(RewriteFunFile ++ ExtraFiles ++ JsFiles, "share/server/main.js"),
ok = Concat(RewriteFunWithASTBypassFile ++ ExtraFiles ++ JsFiles, "share/server/main-ast-bypass.js")
end,
ok = Concat(ExtraFiles ++ CoffeeFiles, "share/server/main-coffee.js"),
ok.