Fix some low hanging inefficiencies in the docs server. Two of the most

expensive operations (by profiling), apart from the template rendering itself,
are calling UnixName (model.py) and removing comments from JSON files
(json_comment_eater.py). This rewrites both and memoizes the former.

BUG=227490

Review URL: https://chromiumcodereview.appspot.com/13599004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@193334 0039d316-1c4b-4281-b951-d872f2087c98

chrome/common/extensions/docs/server2/build_server.py

@@ -15,7 +15,8 @@
 THIRD_PARTY_DIR = os.path.join(SRC_DIR, 'third_party')
 LOCAL_THIRD_PARTY_DIR = os.path.join(sys.path[0], 'third_party')
 TOOLS_DIR = os.path.join(SRC_DIR, 'tools')
-SCHEMA_COMPILER_FILES = ['model.py',
+SCHEMA_COMPILER_FILES = ['memoize.py',
+                         'model.py',
                          'idl_schema.py',
                          'schema_util.py',
                          'json_parse.py']
@@ -64,7 +65,9 @@ def main():
   CopyThirdParty(os.path.join(TOOLS_DIR, 'json_schema_compiler'),
                  'json_schema_compiler',
                  SCHEMA_COMPILER_FILES)
-  CopyThirdParty(TOOLS_DIR, 'json_schema_compiler', ['json_comment_eater.py'])
+  CopyThirdParty(os.path.join(TOOLS_DIR, 'json_comment_eater'),
+                 'json_schema_compiler',
+                 ['json_comment_eater.py'])
   CopyThirdParty(os.path.join(THIRD_PARTY_DIR, 'simplejson'),
                  os.path.join('json_schema_compiler', 'simplejson'),
                  make_init=False)

chrome/common/extensions/docs/server2/compiled_file_system_test.py

@@ -49,13 +49,16 @@ def testIdentityFromFile(self):
 
   def testIdentityFromFileListing(self):
     compiled_fs = _CreateFactory().GetOrCreateIdentity()
-    self.assertEqual({'404.html', 'apps/a11y.html', 'apps/about_apps.html',
-                      'apps/fakedir/file.html',
-                      'extensions/activeTab.html', 'extensions/alarms.html'},
+    self.assertEqual(set(('404.html',
+                          'apps/a11y.html',
+                          'apps/about_apps.html',
+                          'apps/fakedir/file.html',
+                          'extensions/activeTab.html',
+                          'extensions/alarms.html')),
                      set(compiled_fs.GetFromFileListing('/')))
-    self.assertEqual({'a11y.html', 'about_apps.html', 'fakedir/file.html'},
+    self.assertEqual(set(('a11y.html', 'about_apps.html', 'fakedir/file.html')),
                      set(compiled_fs.GetFromFileListing('apps/')))
-    self.assertEqual({'file.html'},
+    self.assertEqual(set(('file.html',)),
                      set(compiled_fs.GetFromFileListing('apps/fakedir')))
 
   def testPopulateNamespace(self):
@@ -92,18 +95,18 @@ def Sleepy(key, val):
   def testCaching(self):
     compiled_fs = _CreateFactory().GetOrCreateIdentity()
     self.assertEqual('404.html contents', compiled_fs.GetFromFile('404.html'))
-    self.assertEqual({'file.html'},
+    self.assertEqual(set(('file.html',)),
                      set(compiled_fs.GetFromFileListing('apps/fakedir')))
 
     compiled_fs._file_system._obj['404.html'] = 'boom'
     compiled_fs._file_system._obj['apps']['fakedir']['boom.html'] = 'blam'
     self.assertEqual('404.html contents', compiled_fs.GetFromFile('404.html'))
-    self.assertEqual({'file.html'},
+    self.assertEqual(set(('file.html',)),
                      set(compiled_fs.GetFromFileListing('apps/fakedir')))
 
     compiled_fs._file_system.IncrementStat()
     self.assertEqual('boom', compiled_fs.GetFromFile('404.html'))
-    self.assertEqual({'file.html', 'boom.html'},
+    self.assertEqual(set(('file.html', 'boom.html')),
                      set(compiled_fs.GetFromFileListing('apps/fakedir')))
 
   def testFailures(self):

chrome/common/extensions/docs/server2/object_store_creator_test.py

@@ -32,10 +32,10 @@ def testWithVersionAndCategory(self):
     self.assertEqual('_FooClass/mat/43', store.namespace)
 
   def testIllegalIinput(self):
-    self.assertRaises(self.creator.Create, category='5')
-    self.assertRaises(self.creator.Create, category='forty2')
-    self.assertRaises(self.creator.Create, version='twenty')
-    self.assertRaises(self.creator.Create, version='7a')
+    self.assertRaises(AssertionError, self.creator.Create, category='5')
+    self.assertRaises(AssertionError, self.creator.Create, category='forty2')
+    self.assertRaises(AssertionError, self.creator.Create, version='twenty')
+    self.assertRaises(AssertionError, self.creator.Create, version='7a')
 
 if __name__ == '__main__':
   unittest.main()

chrome/common/extensions/docs/server2/preview.py

@@ -34,6 +34,7 @@
 import shutil
 from StringIO import StringIO
 import sys
+import time
 import urlparse
 
 import build_server
@@ -96,6 +97,8 @@ def do_GET(self):
            'the server, e.g. apps/storage.html. The path may optionally end '
            'with #n where n is the number of times to render the page before '
            'printing it, e.g. apps/storage.html#50, to use for profiling.')
+  parser.add_option('-t', '--time', action='store_true',
+      help='Print the time taken rendering rather than the result.')
 
   (opts, argv) = parser.parse_args()
 
@@ -117,6 +120,9 @@ def do_GET(self):
       path = opts.render
       extra_iterations = 0
 
+    if opts.time:
+      start_time = time.time()
+
     content, status, headers = _Render(path)
     if status in [301, 302]:
       # Handle a single level of redirection.
@@ -130,9 +136,12 @@ def do_GET(self):
     for _ in range(extra_iterations):
       _Render(path)
 
-    # Static paths will show up as /stable/static/foo but this only makes sense
-    # from a webserver.
-    print(content.replace('/stable/static', 'static'))
+    if opts.time:
+      print('Took %s seconds' % (time.time() - start_time))
+    else:
+      # Static paths will show up as /stable/static/foo but this only makes
+      # sense from a webserver.
+      print(content.replace('/stable/static', 'static'))
     exit()
 
   print('Starting previewserver on port %s' % opts.port)

tools/json_comment_eater/everything.json

@@ -0,0 +1,13 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Test API.
+{ "namespace": "test",
+  "comments": "yo",  // Comments all have a // in them.
+  "strings": "yes",  // Comment with "strings" and " character
+  "escaped\"": "string\"isescaped",
+  "more//": "\"more",
+  "so\\many": "\\\\escapes\\\\\"whoa",
+  "comment//inmiddle": "of string"
+}

tools/json_comment_eater/everything_expected.json

@@ -0,0 +1,13 @@
+
+
+
+
+
+{ "namespace": "test",
+  "comments": "yo",  
+  "strings": "yes",  
+  "escaped\"": "string\"isescaped",
+  "more//": "\"more",
+  "so\\many": "\\\\escapes\\\\\"whoa",
+  "comment//inmiddle": "of string"
+}

tools/json_comment_eater/json_comment_eater.py

@@ -0,0 +1,60 @@
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+'''Utility to remove comments from JSON files so that they can be parsed by
+json.loads.
+'''
+
+def _Rcount(string, chars):
+  '''Returns the number of consecutive characters from |chars| that occur at the
+  end of |string|.
+  '''
+  return len(string) - len(string.rstrip(chars))
+
+def _FindNextToken(string, tokens, start):
+  '''Finds the next token in |tokens| that occurs in |string| from |start|.
+  Returns a tuple (index, token key).
+  '''
+  min_index, min_key = (-1, None)
+  for k in tokens:
+    index = string.find(k, start)
+    if index != -1 and (min_index == -1 or index < min_index):
+      min_index, min_key = (index, k)
+  return (min_index, min_key)
+
+def _ReadString(input, start, output):
+  output.append('"')
+  start_range, end_range = (start, input.find('"', start))
+  # \" escapes the ", \\" doesn't, \\\" does, etc.
+  while (end_range != -1 and
+         _Rcount(input[start_range:end_range], '\\') % 2 == 1):
+    start_range, end_range = (end_range, input.find('"', end_range + 1))
+  if end_range == -1:
+    return start_range + 1
+  output.append(input[start:end_range + 1])
+  return end_range + 1
+
+def _ReadComment(input, start, output):
+  eol_tokens = ('\n', '\r')
+  eol_token_index, eol_token = _FindNextToken(input, eol_tokens, start)
+  if eol_token is None:
+    return len(input)
+  output.append(eol_token)
+  return eol_token_index + len(eol_token)
+
+def Nom(input):
+  token_actions = {
+    '"': _ReadString,
+    '//': _ReadComment,
+  }
+  output = []
+  pos = 0
+  while pos < len(input):
+    token_index, token = _FindNextToken(input, token_actions.keys(), pos)
+    if token is None:
+      output.append(input[pos:])
+      break
+    output.append(input[pos:token_index])
+    pos = token_actions[token](input, token_index + len(token), output)
+  return ''.join(output)

tools/json_comment_eater/json_comment_eater_test.py

@@ -0,0 +1,26 @@
+#!/usr/bin/env python
+# Copyright 2013 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+from json_comment_eater import Nom
+import unittest
+
+class JsonCommentEaterTest(unittest.TestCase):
+  def _Load(self, test_name):
+    '''Loads the input and expected output for |test_name| as given by reading
+    in |test_name|.json and |test_name|_expected.json, and returns the string
+    contents as a tuple in that order.
+    '''
+    def read(file_name):
+      with open(file_name, 'r') as f:
+        return f.read()
+    return [read(pattern % test_name)
+            for pattern in ('%s.json', '%s_expected.json')]
+
+  def testEverything(self):
+    json, expected_json = self._Load('everything')
+    self.assertEqual(expected_json, Nom(json))
+
+if __name__ == '__main__':
+  unittest.main()

tools/json_schema_compiler/json_parse.py

@@ -10,7 +10,8 @@
 _FILE_PATH = os.path.dirname(os.path.realpath(__file__))
 _SYS_PATH = sys.path[:]
 try:
-  _COMMENT_EATER_PATH = os.path.join(_FILE_PATH, os.pardir)
+  _COMMENT_EATER_PATH = os.path.join(
+      _FILE_PATH, os.pardir, 'json_comment_eater')
   sys.path.insert(0, _COMMENT_EATER_PATH)
   import json_comment_eater
 finally:

tools/json_schema_compiler/memoize.py

@@ -0,0 +1,13 @@
+# Copyright 2013 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+def memoize(fn):
+  '''Decorates |fn| to memoize.
+  '''
+  memory = {}
+  def impl(*args):
+    if args not in memory:
+      memory[args] = fn(*args)
+    return memory[args]
+  return impl

tools/json_schema_compiler/model.py

@@ -2,11 +2,10 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-import copy
 import os.path
-import re
 
 from json_parse import OrderedDict
+from memoize import memoize
 
 class ParseException(Exception):
   """Thrown when data in the model is invalid.
@@ -373,15 +372,26 @@ class PropertyType(object):
   BINARY = _PropertyTypeInfo(False, "binary")
   ANY = _PropertyTypeInfo(False, "any")
 
+@memoize
 def UnixName(name):
-  """Returns the unix_style name for a given lowerCamelCase string.
-  """
-  # First replace any lowerUpper patterns with lower_Upper.
-  s1 = re.sub('([a-z])([A-Z])', r'\1_\2', name)
-  # Now replace any ACMEWidgets patterns with ACME_Widgets
-  s2 = re.sub('([A-Z]+)([A-Z][a-z])', r'\1_\2', s1)
-  # Finally, replace any remaining periods, and make lowercase.
-  return s2.replace('.', '_').lower()
+  '''Returns the unix_style name for a given lowerCamelCase string.
+  '''
+  unix_name = []
+  for i, c in enumerate(name):
+    if c.isupper() and i > 0:
+      # Replace lowerUpper with lower_Upper.
+      if name[i - 1].islower():
+        unix_name.append('_')
+      # Replace ACMEWidgets with ACME_Widgets
+      elif i + 1 < len(name) and name[i + 1].islower():
+        unix_name.append('_')
+    if c == '.':
+      # Replace hello.world with hello_world.
+      unix_name.append('_')
+    else:
+      # Everything is lowercase.
+      unix_name.append(c.lower())
+  return ''.join(unix_name)
 
 def _StripNamespace(name, namespace):
   if name.startswith(namespace.name + '.'):

tools/json_to_struct/json_to_struct.py

@@ -54,7 +54,7 @@
 import re
 _script_path = os.path.realpath(__file__)
 
-sys.path.insert(0, os.path.normpath(_script_path + "/../../"))
+sys.path.insert(0, os.path.normpath(_script_path + "/../../json_comment_eater"))
 try:
   import json_comment_eater
 finally: