Merge pull request #14 from couchbase/feature/issue_1375

Fixed https://github.com/couchbase/couchbase-lite-java-core/issues/1375
couchbase · Jul 29, 2016 · 7374ac8 · 7374ac8
2 parents c5d557d + fd32460
commit 7374ac8
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 60 deletions.
diff --git a/src/main/java/com/couchbase/lite/javascript/ReplicationFilterBlockRhino.java b/src/main/java/com/couchbase/lite/javascript/ReplicationFilterBlockRhino.java
@@ -1,17 +1,16 @@
-/**
- * Copyright (c) 2015 Couchbase, Inc All rights reserved.
- *
- * 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
- *
- * http://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.
- */
-
+//
+// Copyright (c) 2016 Couchbase, Inc. All rights reserved.
+//
+// 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
+//
+// http://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.
+//
 package com.couchbase.lite.javascript;
 
 import com.couchbase.lite.ReplicationFilter;
@@ -30,14 +29,21 @@
  * Created by hideki on 10/28/15.
  */
 public class ReplicationFilterBlockRhino implements ReplicationFilter {
-    public static String TAG = "ReplicationFilterBlockRhino";
+    public static String TAG = "JavaScriptEngine";
 
     private static WrapFactory wrapFactory = new CustomWrapFactory();
     private Scriptable scope;
     private GlobalScope globalScope;
     private Function filterFunction;
 
-    public ReplicationFilterBlockRhino(String src){
+    // NOTE: Scope is sharable with multiple threads, it seems `Function` is not.
+    //       Compiling javascript codes for every request makes slow.
+    //       It is reason current code base re-use compiled Function.
+    //       Instead of compiling for every request, use `synchronized` to protect Function
+    //       https://developer.mozilla.org/en-US/docs/Mozilla/Projects/Rhino/Scopes_and_Contexts
+    private final Object lockFunction = new Object();
+
+    public ReplicationFilterBlockRhino(String src) {
         org.mozilla.javascript.Context ctx = org.mozilla.javascript.Context.enter();
         try {
             ctx.setOptimizationLevel(-1);
@@ -56,19 +62,18 @@ public boolean filter(SavedRevision revision, Map<String, Object> params) {
         try {
             ctx.setOptimizationLevel(-1);
             ctx.setWrapFactory(wrapFactory);
-
             Scriptable localScope = ctx.newObject(scope);
             localScope.setPrototype(scope);
             localScope.setParentScope(null);
-
             Object jsDocument = org.mozilla.javascript.Context.javaToJS(revision.getProperties(), localScope);
             Object jsParams = org.mozilla.javascript.Context.javaToJS(params, localScope);
 
             try {
-                Object result = filterFunction.call(ctx, localScope, null, new Object[]{jsDocument, jsParams});
-                return ((Boolean)result).booleanValue();
+                synchronized (lockFunction) {
+                    Object result = filterFunction.call(ctx, localScope, null, new Object[]{jsDocument, jsParams});
+                    return ((Boolean) result).booleanValue();
+                }
             } catch (org.mozilla.javascript.RhinoException e) {
-                // Error in the JavaScript view - CouchDB swallows  the error and tries the next document
                 Log.e(TAG, "Error in filterFunction.call()", e);
                 return false;
             }

diff --git a/src/main/java/com/couchbase/lite/javascript/ViewMapBlockRhino.java b/src/main/java/com/couchbase/lite/javascript/ViewMapBlockRhino.java
@@ -1,22 +1,23 @@
-/**
- * Copyright (c) 2015 Couchbase, Inc All rights reserved.
- *
- * 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
- *
- * http://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.
- */
+//
+// Copyright (c) 2016 Couchbase, Inc. All rights reserved.
+//
+// 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
+//
+// http://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.
+//
 package com.couchbase.lite.javascript;
 
 import com.couchbase.lite.Emitter;
 import com.couchbase.lite.Mapper;
 import com.couchbase.lite.javascript.scopes.MapGlobalScope;
 import com.couchbase.lite.javascript.wrapper.CustomWrapFactory;
+import com.couchbase.lite.util.Log;
 
 import org.mozilla.javascript.Function;
 import org.mozilla.javascript.Scriptable;
@@ -25,16 +26,22 @@
 import java.util.Map;
 
 class ViewMapBlockRhino implements Mapper {
+    private final static String TAG = "JavaScriptEngine";
 
     private static WrapFactory wrapFactory = new CustomWrapFactory();
     private Scriptable globalScope;
     private MapGlobalScope mapGlobalScope;
     private Function mapFunction;
 
-    public ViewMapBlockRhino(String src) {
+    // NOTE: Scope is sharable with multiple threads, it seems `Function` is not.
+    //       Compiling javascript codes for every request makes slow.
+    //       It is reason current code base re-use compiled Function.
+    //       Instead of compiling for every request, use `synchronized` to protect Function
+    //       https://developer.mozilla.org/en-US/docs/Mozilla/Projects/Rhino/Scopes_and_Contexts
+    private final Object lockFunction = new Object();
 
+    public ViewMapBlockRhino(String src) {
         org.mozilla.javascript.Context ctx = org.mozilla.javascript.Context.enter();
-
         try {
             ctx.setOptimizationLevel(-1);
             ctx.setWrapFactory(wrapFactory);
@@ -48,7 +55,6 @@ public ViewMapBlockRhino(String src) {
 
     @Override
     public void map(Map<String, Object> document, Emitter emitter) {
-
         mapGlobalScope.setEmitter(emitter);
 
         org.mozilla.javascript.Context ctx = org.mozilla.javascript.Context.enter();
@@ -63,9 +69,11 @@ public void map(Map<String, Object> document, Emitter emitter) {
             Object jsDocument = org.mozilla.javascript.Context.javaToJS(document, localScope);
 
             try {
-                mapFunction.call(ctx, localScope, null, new Object[]{jsDocument});
+                synchronized (lockFunction) {
+                    mapFunction.call(ctx, localScope, null, new Object[]{jsDocument});
+                }
             } catch (org.mozilla.javascript.RhinoException e) {
-                // Error in the JavaScript view - CouchDB swallows  the error and tries the next document
+                Log.e(TAG, "Error in calling JavaScript map function", e);
                 return;
             }
         } finally {

diff --git a/src/main/java/com/couchbase/lite/javascript/ViewReduceBlockRhino.java b/src/main/java/com/couchbase/lite/javascript/ViewReduceBlockRhino.java
@@ -1,21 +1,22 @@
-/**
- * Copyright (c) 2015 Couchbase, Inc All rights reserved.
- *
- * 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
- *
- * http://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.
- */
+//
+// Copyright (c) 2016 Couchbase, Inc. All rights reserved.
+//
+// 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
+//
+// http://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.
+//
 package com.couchbase.lite.javascript;
 
 import com.couchbase.lite.Reducer;
 import com.couchbase.lite.javascript.scopes.ReduceGlobalScope;
 import com.couchbase.lite.javascript.wrapper.CustomWrapFactory;
+import com.couchbase.lite.util.Log;
 
 import org.mozilla.javascript.Function;
 import org.mozilla.javascript.Scriptable;
@@ -25,20 +26,27 @@
 import java.util.List;
 
 class ViewReduceBlockRhino implements Reducer {
+    private final static String TAG = "JavaScriptEngine";
 
     private static WrapFactory wrapFactory = new CustomWrapFactory();
     private final ReduceGlobalScope reduceGlobalScope;
     private final ScriptableObject globalScope;
     private final Function reduceFunction;
     private final NativReduceFunctions nativeReduce;
 
+    // NOTE: Scope is sharable with multiple threads, it seems `Function` is not.
+    //       Compiling javascript codes for every request makes slow.
+    //       It is reason current code base re-use compiled Function.
+    //       Instead of compiling for every request, use `synchronized` to protect Function
+    //       https://developer.mozilla.org/en-US/docs/Mozilla/Projects/Rhino/Scopes_and_Contexts
+    private final Object lockFunction = new Object();
+
     public ViewReduceBlockRhino(String src) {
 
         nativeReduce = NativReduceFunctions.fromKey(src);
 
         if (nativeReduce == NativReduceFunctions.DEFAULT) {
             org.mozilla.javascript.Context ctx = org.mozilla.javascript.Context.enter();
-
             try {
                 ctx.setOptimizationLevel(-1);
                 ctx.setWrapFactory(wrapFactory);
@@ -58,7 +66,6 @@ public ViewReduceBlockRhino(String src) {
 
     @Override
     public Object reduce(List<Object> keys, List<Object> values, boolean reReduce) {
-
         switch (nativeReduce) {
             case SUM:
                 return nativeSum(keys, values, reReduce);
@@ -76,16 +83,18 @@ public Object reduce(List<Object> keys, List<Object> values, boolean reReduce) {
                     localScope.setParentScope(null);
 
                     Object[] args = new Object[3];
-
                     args[0] = org.mozilla.javascript.Context.javaToJS(keys, localScope);
                     args[1] = org.mozilla.javascript.Context.javaToJS(values, localScope);
                     args[2] = org.mozilla.javascript.Context.javaToJS(reReduce, localScope);
 
-                    return reduceFunction.call(ctx, localScope, null, args);
-
-                } catch (org.mozilla.javascript.RhinoException e) {
-                    // TODO check couchdb behaviour on error in reduce function
-                    return null;
+                    try {
+                        synchronized (lockFunction) {
+                            return reduceFunction.call(ctx, localScope, null, args);
+                        }
+                    } catch (org.mozilla.javascript.RhinoException e) {
+                        Log.e(TAG, "Error in calling JavaScript reduce function", e);
+                        return null;
+                    }
                 } finally {
                     org.mozilla.javascript.Context.exit();
                 }