[GitHub] [arrow] pitrou commented on a diff in pull request #14100: ARROW-4709: [C++] Optimize for ordered JSON fields

[GitBox <gi...@apache.org>]

pitrou commented on code in PR #14100:
URL: https://github.com/apache/arrow/pull/14100#discussion_r1023065903


##########
cpp/src/arrow/json/parser.cc:
##########
@@ -320,43 +341,75 @@ class RawArrayBuilder<Kind::kArray> {
 template <>
 class RawArrayBuilder<Kind::kObject> {
  public:
-  explicit RawArrayBuilder(MemoryPool* pool) : null_bitmap_builder_(pool) {}
+  explicit RawArrayBuilder(BuildContext* context)
+      : context_(context), null_bitmap_builder_(context->pool()) {}
 
   Status Append() { return null_bitmap_builder_.Append(true); }
 
   Status AppendNull() { return null_bitmap_builder_.Append(false); }
 
   Status AppendNull(int64_t count) { return null_bitmap_builder_.Append(count, false); }
 
-  std::string FieldName(int i) const {
-    for (const auto& name_index : name_to_index_) {
-      if (name_index.second == i) {
-        return name_index.first;
-      }
-    }
-    return "";
+  int FindFieldIndex(std::string_view name) const {
+    auto it = name_to_index_.find(name);
+    return it != name_to_index_.end() ? it->second : -1;
   }
 
-  int GetFieldIndex(const std::string& name) const {
-    auto it = name_to_index_.find(name);
-    if (it == name_to_index_.end()) {
+  int GetFieldIndex(std::string_view name) {
+    if (ARROW_PREDICT_FALSE(num_fields() == 0)) {
       return -1;
     }
-    return it->second;
+
+    if (skip_index_queue_) {
+      return FindFieldIndex(name);
+    }
+
+    // Field ordering has been predictable thus far, so check the LRU index first
+    auto index = index_queue_.front();
+    if (ARROW_PREDICT_TRUE(name == field_infos_[index].name)) {
+      // Front index was a match - rotate the queue
+      index_queue_.splice(index_queue_.cend(), index_queue_, index_queue_.cbegin());

Review Comment:
   Looking at the libstdc++ source code, it looks like `list::splice` might not be as trivial. In particular, it will call a non-inline method `std::__detail::_List_node_base::_M_transfer` to handle the actual pointer splicing.
   
   I would therefore suggest using a `std::vector<int>` instead, and track the current index in the vector. It will also reduce pointer chasing.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org