From: Jakub Skowron <j.skowron@samsung.com>
Date: Wed, 10 Jan 2018 06:54:35 +0000 (+0100)
Subject: [Filesystem] Read file performace improvement (binary data through UTF-8)
X-Git-Tag: submit/tizen_3.0/20180122.134630~4^2
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=f95fafc2ed357c0cc33e6e8658cee44ae15d5e14;p=platform%2Fcore%2Fapi%2Fwebapi-plugins.git

[Filesystem] Read file performace improvement (binary data through UTF-8)

Improve performance of readBytes, readBase64 by pushing binary data
through UTF-8 null-terminated string.
Byte 0x00 is encoded as U+0100.

Removed not used functions in FilesystemFile. Improved performance of
readAsText.

[Verification] TCT pass rate is 100%
    tct-filesystem-tizen-tests

Change-Id: I41fe2f9c7855721b38ad4bbf268e8eb7851b10eb
Signed-off-by: Jakub Skowron <j.skowron@samsung.com>
---

diff --git a/src/filesystem/filesystem_file.cc b/src/filesystem/filesystem_file.cc
index 7172f3d8..11747476 100644
--- a/src/filesystem/filesystem_file.cc
+++ b/src/filesystem/filesystem_file.cc
@@ -162,112 +162,6 @@ FilesystemFile::FilesystemFile(const std::string& path_) : path(path_) {
   ScopeLogger();
 }
 
-bool FilesystemFile::Read(FilesystemBuffer* data, size_t offset, size_t length) {
-  ScopeLogger();
-  if (!data) {
-    LoggerE("Missing output buffer");
-    return false;
-  }
-
-  data->resize(length);
-  FILE* file = fopen(path.c_str(), "r");
-  if (!file) {
-    LoggerE("Cannot open file %s to read!", path.c_str());
-    return false;
-  }
-  SCOPE_EXIT {
-    int status = fclose(file);
-    if (status) {
-      LoggerE("Cannot close file!");
-    }
-  };
-  int status;
-  status = fseek(file, offset, SEEK_SET);
-  if (status) {
-    LoggerE("Cannot perform seek!");
-    return false;
-  }
-
-  size_t readed = 0;
-  uint8_t* data_p = data->data();
-  size_t data_size = length;
-  while (readed < data->size()) {
-    size_t part = fread(data_p, 1, data_size, file);
-
-    readed += part;
-    data_p += part;
-    data_size -= part;
-
-    LoggerD("Readed part %li bytes", readed);
-
-    if (ferror(file)) {
-      LoggerE("Error during file write!");
-      return false;
-    }
-
-    if (feof(file)) {
-      LoggerD("File is at end before buffer is filled. Finish.");
-      break;
-    }
-  }
-  LoggerD("Readed %li bytes", readed);
-  data->resize(readed);
-  return true;
-}
-
-bool FilesystemFile::ReadString(std::vector<char>* data, size_t offset, size_t length) {
-  ScopeLogger();
-  if (!data) {
-    LoggerE("Missing output buffer");
-    return false;
-  }
-
-  data->resize(length);
-  FILE* file = fopen(path.c_str(), "r");
-  if (!file) {
-    LoggerE("Cannot open file %s to read!", path.c_str());
-    return false;
-  }
-  SCOPE_EXIT {
-    int status = fclose(file);
-    if (status) {
-      LoggerE("Cannot close file!");
-    }
-  };
-  int status;
-  status = fseek(file, offset, SEEK_SET);
-  if (status) {
-    LoggerE("Cannot perform seek!");
-    return false;
-  }
-
-  size_t readed = 0;
-  char* data_p = data->data();
-  size_t data_size = length;
-  while (readed < data->size()) {
-    size_t part = fread(data_p, 1, data_size, file);
-
-    readed += part;
-    data_p += part;
-    data_size -= part;
-
-    LoggerD("Readed part %u bytes", readed);
-
-    if (ferror(file)) {
-      LoggerE("Error during file write!");
-      return false;
-    }
-
-    if (feof(file)) {
-      LoggerD("File is at end before buffer is filled. Finish.");
-      break;
-    }
-  }
-  LoggerD("Readed %u bytes", readed);
-  data->resize(readed);
-  return true;
-}
-
 bool FilesystemFile::Write(const FilesystemBuffer& data, size_t offset, bool rewrite) {
   ScopeLogger();
 
@@ -292,7 +186,7 @@ bool FilesystemFile::Write(const FilesystemBuffer& data, size_t offset, bool rew
 
   int status;
   status = fseek(file, offset, SEEK_SET);
-  LoggerD("Offset is %li, writing %i bytes", offset, data.size());
+  LoggerD("Offset is %u, writing %i bytes", offset, data.size());
   if (status) {
     LoggerE("Cannot perform seek!");
     return false;
@@ -325,7 +219,7 @@ bool FilesystemFile::Write(const FilesystemBuffer& data, size_t offset, bool rew
     LoggerE("Cannot sync file!");
     return false;
   }
-  LoggerD("Written %li bytes", written);
+  LoggerD("Written %u bytes", written);
 
   return true;
 }
diff --git a/src/filesystem/filesystem_file.h b/src/filesystem/filesystem_file.h
index 1e2d5c8b..e8dce335 100644
--- a/src/filesystem/filesystem_file.h
+++ b/src/filesystem/filesystem_file.h
@@ -44,8 +44,6 @@ class FilesystemFile {
  public:
   FilesystemFile(const std::string& path_);
 
-  bool Read(FilesystemBuffer* data, size_t offset, size_t length);
-  bool ReadString(std::vector<char>* data, size_t offset, size_t length);
   bool Write(const FilesystemBuffer& data, size_t offset, bool rewrite);
   bool WriteString(const std::string& data, size_t offset, bool rewrite);
 };
diff --git a/src/filesystem/filesystem_instance.cc b/src/filesystem/filesystem_instance.cc
index e32021ae..848c6dce 100644
--- a/src/filesystem/filesystem_instance.cc
+++ b/src/filesystem/filesystem_instance.cc
@@ -16,11 +16,15 @@
 
 #include "filesystem/filesystem_instance.h"
 
+#include <cstdint>
 #include <functional>
+#include <stdexcept>
 
+#include <sys/stat.h>
 #include "common/logger.h"
 #include "common/picojson.h"
 #include "common/platform_exception.h"
+#include "common/scope_exit.h"
 #include "common/task-queue.h"
 #include "common/tools.h"
 #include "filesystem_manager.h"
@@ -43,29 +47,26 @@ FilesystemInstance::FilesystemInstance() {
   using std::placeholders::_2;
 
 #define REGISTER_SYNC(c, x) RegisterSyncHandler(c, std::bind(&FilesystemInstance::x, this, _1, _2));
-#define REGISTER_ASYNC(c, x) \
-  RegisterSyncHandler(c, std::bind(&FilesystemInstance::x, this, _1, _2));
-
-  REGISTER_ASYNC("File_stat", FileStat);
+  REGISTER_SYNC("File_stat", FileStat);
   REGISTER_SYNC("File_statSync", FileStatSync);
   REGISTER_SYNC("File_createSync", FileCreateSync);
-  REGISTER_ASYNC("File_readDir", ReadDir);
-  REGISTER_ASYNC("File_rename", FileRename);
-  REGISTER_SYNC("File_readSync", FileReadSync);
-  REGISTER_ASYNC("File_write", FileWrite);
+  REGISTER_SYNC("File_readDir", ReadDir);
+  REGISTER_SYNC("File_rename", FileRename);
+  REGISTER_SYNC("File_readBytes", FileReadBytes);
+  REGISTER_SYNC("File_readString", FileReadString);
+  REGISTER_SYNC("File_write", FileWrite);
   REGISTER_SYNC("File_writeSync", FileWriteSync);
   REGISTER_SYNC("Filesystem_fetchVirtualRoots", FilesystemFetchVirtualRoots);
   REGISTER_SYNC("FileSystemManager_addStorageStateChangeListener", StartListening);
   REGISTER_SYNC("FileSystemManager_removeStorageStateChangeListener", StopListening);
   REGISTER_SYNC("FileSystemManager_fetchStorages", FileSystemManagerFetchStorages);
-  REGISTER_ASYNC("FileSystemManager_mkdir", FileSystemManagerMakeDirectory);
+  REGISTER_SYNC("FileSystemManager_mkdir", FileSystemManagerMakeDirectory);
   REGISTER_SYNC("FileSystemManager_mkdirSync", FileSystemManagerMakeDirectorySync);
-  REGISTER_ASYNC("File_unlinkFile", UnlinkFile);
-  REGISTER_ASYNC("File_removeDirectory", RemoveDirectory);
-  REGISTER_ASYNC("File_copyTo", CopyTo);
+  REGISTER_SYNC("File_unlinkFile", UnlinkFile);
+  REGISTER_SYNC("File_removeDirectory", RemoveDirectory);
+  REGISTER_SYNC("File_copyTo", CopyTo);
   REGISTER_SYNC("FileSystemManager_getCanonicalPath", FileSystemManagerGetCanonicalPath);
 #undef REGISTER_SYNC
-#undef REGISTER_ASYNC
   FilesystemManager::GetInstance().AddListener(this);
 }
 
@@ -135,31 +136,122 @@ void FilesystemInstance::FileRename(const picojson::value& args, picojson::objec
       std::bind(&FilesystemManager::Rename, &fsm, oldPath, newPath, onSuccess, onError));
 }
 
-void FilesystemInstance::FileReadSync(const picojson::value& args, picojson::object& out) {
+static constexpr std::size_t NPOS = (std::size_t)(-1);
+/**
+ * Returns a buffer. If length is NPOS, then it reads whole file, up to the end.
+ * On failure throws std::runtime_error
+ */
+static std::vector<std::uint8_t> read_file(std::string path, std::size_t offset,
+                                           std::size_t length = NPOS) {
+  ScopeLogger();
+
+  FILE* file = std::fopen(path.c_str(), "r");
+  if (!file) {
+    throw std::runtime_error("cannot open file to read");
+  }
+
+  SCOPE_EXIT {
+    int status = std::fclose(file);
+    if (status) {
+      LoggerE("Cannot close file!");
+    }
+  };
+
+  if (std::fseek(file, offset, SEEK_SET) != 0) {
+    throw std::runtime_error("cannot perform seek");
+  }
+
+  // By default reads whole file. Get the file size.
+  if (length == NPOS) {
+    struct ::stat buf;
+    if (::fstat(::fileno(file), &buf) != 0) {
+      throw std::runtime_error("cannot fstat");
+    }
+    length = buf.st_size - offset;
+  }
+
+  std::vector<std::uint8_t> out_buf(length);
+  std::uint8_t* data_p = out_buf.data();
+  std::uint8_t* end_p = data_p + length;
+  while (data_p != end_p) {
+    data_p += std::fread(data_p, 1, end_p - data_p, file);
+
+    if (std::ferror(file)) {
+      throw std::runtime_error("error during file read");
+    }
+
+    if (std::feof(file)) {
+      LoggerD("File is at end before buffer is filled. Finish.");
+      break;
+    }
+  }
+  return out_buf;
+}
+
+/* str should be reference to (possibly empty) std::string */
+static void encode_binary_in_string(const std::vector<std::uint8_t>& buf, std::string& str) {
+  ScopeLogger();
+  str.reserve(str.size() + buf.size());
+
+  for (std::uint8_t byte : buf) {
+    if (byte >= 128) {
+      str += 0xC0 | (byte >> 6);
+      str += 0x80 | (byte & 0x3F);
+    } else if (byte > 0) {
+      str += byte;
+    } else {
+      // zero as codepoint U+0100
+      str += 0xC4;
+      str += 0x80;
+    }
+  }
+}
+
+void FilesystemInstance::FileReadString(const picojson::value& args, picojson::object& out) {
+  ScopeLogger();
+  CHECK_PRIVILEGE_ACCESS(kPrivilegeFilesystemRead, &out);
+  CHECK_EXIST(args, "location", out)
+  CHECK_EXIST(args, "offset", out)
+  CHECK_EXIST(args, "encoding", out)
+
+  const std::string& location = args.get("location").get<std::string>();
+  size_t offset = static_cast<size_t>(args.get("offset").get<double>());
+  size_t length = args.contains("length") ? (size_t)args.get("length").get<double>() : NPOS;
+
+  try {
+    std::vector<std::uint8_t> out_data = read_file(location, offset, length);
+    out_data.push_back('\0');
+    const char* str = (const char*)out_data.data();
+
+    // TODO: support iso-8859-1 (latin-1) encoding
+    ReportSuccess(picojson::value{str}, out);
+  } catch (std::runtime_error& e) {
+    LoggerE("Cannot read file %s, cause: %s", location.c_str(), e.what());
+    PrepareError(FilesystemError::Other, out);
+  }
+}
+
+void FilesystemInstance::FileReadBytes(const picojson::value& args, picojson::object& out) {
   ScopeLogger();
   CHECK_PRIVILEGE_ACCESS(kPrivilegeFilesystemRead, &out);
   CHECK_EXIST(args, "location", out)
   CHECK_EXIST(args, "offset", out)
   CHECK_EXIST(args, "length", out)
-  CHECK_EXIST(args, "isString", out)
 
   const std::string& location = args.get("location").get<std::string>();
   size_t offset = static_cast<size_t>(args.get("offset").get<double>());
   size_t length = static_cast<size_t>(args.get("length").get<double>());
-  bool is_string = static_cast<bool>(args.get("isString").get<bool>());
-
-  auto onSuccess = [this, &out](const std::string& data) {
-    ScopeLogger("Entered into asynchronous function, onSuccess");
-    ReportSuccess(picojson::value(data), out);
-  };
 
-  auto onError = [this, &out](FilesystemError e) {
-    ScopeLogger("Entered into asynchronous function, onError");
-    PrepareError(e, out);
-  };
+  try {
+    std::vector<std::uint8_t> out_data = read_file(location, offset, length);
 
-  FilesystemManager::GetInstance().FileRead(location, offset, length, is_string, onSuccess,
-                                            onError);
+    out["result"] = picojson::value(picojson::string_type, true);
+    encode_binary_in_string(out_data, out["result"].get<std::string>());
+    ReportSuccess(out);
+  } catch (std::runtime_error& e) {
+    LoggerE("Cannot read file %s, cause: %s", location.c_str(), e.what());
+    PrepareError(FilesystemError::Other, out);
+  }
 }
 
 void FilesystemInstance::FileWrite(const picojson::value& args, picojson::object& out) {
diff --git a/src/filesystem/filesystem_instance.h b/src/filesystem/filesystem_instance.h
index 22123232..da21bc74 100644
--- a/src/filesystem/filesystem_instance.h
+++ b/src/filesystem/filesystem_instance.h
@@ -35,8 +35,8 @@ class FilesystemInstance : public common::ParsedInstance, FilesystemStateChangeL
   void FileRename(const picojson::value& args, picojson::object& out);
   void FileStat(const picojson::value& args, picojson::object& out);
   void FileStatSync(const picojson::value& args, picojson::object& out);
-  void FileRead(const picojson::value& args, picojson::object& out);
-  void FileReadSync(const picojson::value& args, picojson::object& out);
+  void FileReadString(const picojson::value& args, picojson::object& out);
+  void FileReadBytes(const picojson::value& args, picojson::object& out);
   void FileWrite(const picojson::value& args, picojson::object& out);
   void FileWriteSync(const picojson::value& args, picojson::object& out);
   void FilesystemFetchVirtualRoots(const picojson::value& args, picojson::object& out);
diff --git a/src/filesystem/filesystem_manager.cc b/src/filesystem/filesystem_manager.cc
index 54fcffd8..0836c767 100644
--- a/src/filesystem/filesystem_manager.cc
+++ b/src/filesystem/filesystem_manager.cc
@@ -397,35 +397,6 @@ void FilesystemManager::RemoveDirectory(const std::string& path,
   return;
 }
 
-void FilesystemManager::FileRead(const std::string& path, size_t offset, size_t length,
-                                 bool is_string,
-                                 const std::function<void(const std::string&)>& success_cb,
-                                 const std::function<void(FilesystemError)>& error_cb) {
-  ScopeLogger();
-  FilesystemFile file(path);
-
-  // Encode data if its type is not string
-  if (!is_string) {
-    FilesystemBuffer buffer;
-    if (!file.Read(&buffer, offset, length)) {
-      LoggerE("Cannot read file %s", path.c_str());
-      error_cb(FilesystemError::Other);
-      return;
-    }
-    std::string out_data = buffer.EncodeData();
-    success_cb(out_data);
-  } else {
-    std::vector<char> buffer(length);
-    if (!file.ReadString(&buffer, offset, length)) {
-      LoggerE("Cannot read file %s", path.c_str());
-      error_cb(FilesystemError::Other);
-      return;
-    }
-    std::string out_data(buffer.begin(), buffer.end());
-    success_cb(out_data);
-  }
-}
-
 void FilesystemManager::FileWrite(const std::string& path, const std::string& data, size_t offset,
                                   bool rewrite, bool is_string,
                                   const std::function<void()>& success_cb,
diff --git a/src/filesystem/filesystem_manager.h b/src/filesystem/filesystem_manager.h
index f4e8c726..1f2fdbb4 100644
--- a/src/filesystem/filesystem_manager.h
+++ b/src/filesystem/filesystem_manager.h
@@ -83,10 +83,6 @@ class FilesystemManager {
   void RemoveDirectory(const std::string& path, const std::function<void()>& success_cb,
                        const std::function<void(FilesystemError)>& error_cb);
 
-  void FileRead(const std::string& path, size_t offset, size_t length, bool is_string,
-                const std::function<void(const std::string&)>& success_cb,
-                const std::function<void(FilesystemError)>& error_cb);
-
   void FileWrite(const std::string& path, const std::string& data, size_t offset, bool rewrite,
                  bool is_string, const std::function<void()>& success_cb,
                  const std::function<void(FilesystemError)>& error_cb);
diff --git a/src/filesystem/js/file.js b/src/filesystem/js/file.js
index a55531d5..524d7b6a 100644
--- a/src/filesystem/js/file.js
+++ b/src/filesystem/js/file.js
@@ -227,7 +227,7 @@ var Encoding = {
   'iso-8859-1': 'iso-8859-1',
   'sjis': 'sjis',  // backward compatibility
   'null': 'utf-8',
-  'undefined': 'utf-8'
+  'undefined': 'utf-8'  // default value
 };
 
 function _checkEncoding(encoding) {
@@ -316,32 +316,23 @@ function readAsText() {
   var data = {
     location: commonFS_.toRealPath(this.fullPath),
     offset: 0,
-    length: 65536,
-    encoding: args.encoding,
-    isString: true
+    encoding: args.encoding
   };
 
   function readFile() {
-    var result, encoded, buffers = [];
-
-    var output = "";
-    do {
-      result = native_.callSync('File_readSync', data);
-      if (native_.isFailure(result)) {
-        setTimeout(function() {
-          native_.callIfPossible(args.onerror, native_.getErrorObject(result));
-        }, 0);
-        return;
-      }
-      result = native_.getResultObject(result);
-      if (result.length) {
-        data.offset += data.length;
-      }
-      output = output + result;
-    } while (result.length);
+    var result;
+
+    result = native_.callSync('File_readString', data);
+    if (native_.isFailure(result)) {
+      setTimeout(function() {
+        native_.callIfPossible(args.onerror, native_.getErrorObject(result));
+      }, 0);
+      return;
+    }
+    result = native_.getResultObject(result);
 
     setTimeout(function() {
-      native_.callIfPossible(args.onsuccess, output);
+      native_.callIfPossible(args.onsuccess, result);
     }, 0);
   }
 
diff --git a/src/filesystem/js/file_stream.js b/src/filesystem/js/file_stream.js
index 2eb097e4..49f6bba6 100644
--- a/src/filesystem/js/file_stream.js
+++ b/src/filesystem/js/file_stream.js
@@ -134,12 +134,12 @@ function read() {
 
   var data = {
     location: commonFS_.toRealPath(this._file.fullPath),
+    encoding: this._encoding,
     offset: this.position || 0,
     length: args.charCount > _count ? _count : args.charCount,
-    isString: true
   };
 
-  var result = native_.callSync('File_readSync', data);
+  var result = native_.callSync('File_readString', data);
   if (native_.isFailure(result)) {
     throw new WebAPIException(WebAPIException.IO_ERR, 'Could not read');
   }
@@ -160,6 +160,17 @@ FileStream.prototype.read = function() {
   return read.apply(this, arguments);
 };
 
+/* returns array of numbers */
+function string_to_binary( str ) {
+  var output = [];
+  var len = str.length;
+  for( var i = 0; i < len; i++ ) {
+    // decode unicode codepoint U+0100 as zero byte
+    output.push( str[i] == '\u0100' ? 0 : str.charCodeAt(i) );
+  }
+  return output;
+}
+
 function readBytes() {
   var args = validator_.validateArgs(arguments, [
     {
@@ -181,16 +192,15 @@ function readBytes() {
   var data = {
     location: commonFS_.toRealPath(this._file.fullPath),
     offset: this.position || 0,
-    length: args.byteCount > _count ? _count : args.byteCount,
-    isString: false
+    length: (args.byteCount > _count ? _count : args.byteCount)
   };
 
-  var result = native_.callSync('File_readSync', data);
+  var result = native_.callSync('File_readBytes', data);
   if (native_.isFailure(result)) {
     throw new WebAPIException(WebAPIException.INVALID_VALUES_ERR, 'Could not read');
   }
 
-  var decoded = Base64.decode(native_.getResultObject(result));
+  var decoded = string_to_binary( native_.getResultObject(result) );
 
   if (decoded.length) {
     can_change_size = true;
@@ -207,63 +217,8 @@ FileStream.prototype.readBytes = function() {
   return readBytes.apply(this, arguments);
 };
 
-function readBase64() {
-  var args = validator_.validateArgs(arguments, [
-    {
-      name: 'byteCount',
-      type: types_.LONG
-    }
-  ]);
-
-  _checkClosed(this);
-  _checkReadAccess(this._mode);
-
-  if (!arguments.length) {
-    throw new WebAPIException(WebAPIException.TYPE_MISMATCH_ERR,
-        'Argument "byteCount" missing');
-  }
-  if (type_.isString(arguments[0]) && !arguments[0].length) {
-    throw new WebAPIException(WebAPIException.INVALID_VALUES_ERR,
-        'Argument "byteCount" must be a number');
-  }
-  if (!type_.isNumber(arguments[0])) {
-    throw new WebAPIException(WebAPIException.TYPE_MISMATCH_ERR,
-        'Argument "byteCount" must be a number');
-  }
-  if (args.byteCount <= 0) {
-    throw new WebAPIException(WebAPIException.TYPE_MISMATCH_ERR,
-        'Argument "byteCount" must be greater than 0');
-  }
-
-  var _count = this.bytesAvailable;
-
-  var data = {
-    location: commonFS_.toRealPath(this._file.fullPath),
-    offset: this.position || 0,
-    length: args.byteCount > _count ? _count : args.byteCount,
-    isString: false
-  };
-
-  var result = native_.callSync('File_readSync', data);
-  if (native_.isFailure(result)) {
-    throw new WebAPIException(WebAPIException.INVALID_VALUES_ERR, 'Could not read');
-  }
-  var encoded = native_.getResultObject(result);
-  var decoded = Base64.decode(encoded);
-
-  if (decoded.length) {
-    can_change_size = true;
-    this.position += decoded.length;
-    can_change_size = false;
-  } else {
-    this.position += 1;   // Set EOF
-  }
-
-  return encoded;
-};
-
 FileStream.prototype.readBase64 = function() {
-  return readBase64.apply(this, arguments);
+  return Base64.encode(readBytes.apply(this, arguments));
 }
 
 function write() {