Skip setIgnoredValues when there are no ignored values (#108179)

The current logic iterates over all fields in every doc to check for ignored fields, even when no fields were retrieved from _ignored_source. The fix should address the performance regression we noticed, where `setIgnoredValues` dominates cpu time: <img width="1417" alt="Screenshot 2024-05-02 at 12 11 01" src="https://github.com/elastic/elasticsearch/assets/131142368/66c5f944-21d4-41b1-955f-9ca434151cbd"> Related to #106825
elastic · May 2, 2024 · 2c2a85b · 2c2a85b
1 parent 78b0752
commit 2c2a85b
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 3 deletions.
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java
@@ -820,7 +820,10 @@ public void write(XContentBuilder b) throws IOException {
 
         @Override
         public boolean setIgnoredValues(Map<String, List<IgnoredSourceFieldMapper.NameValue>> objectsWithIgnoredFields) {
-            ignoredValues = objectsWithIgnoredFields.get(name());
+            if (objectsWithIgnoredFields == null || objectsWithIgnoredFields.isEmpty()) {
+                return false;
+            }
+            ignoredValues = objectsWithIgnoredFields.remove(name());
             hasValue |= ignoredValues != null;
             for (SourceLoader.SyntheticFieldLoader loader : fields) {
                 hasValue |= loader.setIgnoredValues(objectsWithIgnoredFields);

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java
@@ -126,7 +126,7 @@ private SyntheticLeaf(SyntheticFieldLoader loader, SyntheticFieldLoader.DocValue
             @Override
             public Source source(LeafStoredFieldLoader storedFieldLoader, int docId) throws IOException {
                 // Maps the names of existing objects to lists of ignored fields they contain.
-                Map<String, List<IgnoredSourceFieldMapper.NameValue>> objectsWithIgnoredFields = new HashMap<>();
+                Map<String, List<IgnoredSourceFieldMapper.NameValue>> objectsWithIgnoredFields = null;
 
                 for (Map.Entry<String, List<Object>> e : storedFieldLoader.storedFields().entrySet()) {
                     SyntheticFieldLoader.StoredFieldLoader loader = storedFieldLoaders.get(e.getKey());
@@ -135,12 +135,17 @@ public Source source(LeafStoredFieldLoader storedFieldLoader, int docId) throws
                     }
                     if (IgnoredSourceFieldMapper.NAME.equals(e.getKey())) {
                         for (Object value : e.getValue()) {
+                            if (objectsWithIgnoredFields == null) {
+                                objectsWithIgnoredFields = new HashMap<>();
+                            }
                             IgnoredSourceFieldMapper.NameValue nameValue = IgnoredSourceFieldMapper.decode(value);
                             objectsWithIgnoredFields.computeIfAbsent(nameValue.getParentFieldName(), k -> new ArrayList<>()).add(nameValue);
                         }
                     }
                 }
-                loader.setIgnoredValues(objectsWithIgnoredFields);
+                if (objectsWithIgnoredFields != null) {
+                    loader.setIgnoredValues(objectsWithIgnoredFields);
+                }
                 if (docValuesLoader != null) {
                     docValuesLoader.advanceToDoc(docId);
                 }