Repository: saltstack/salt
Pull request: Fix serious performance issues with the file.tidied module
### What does this PR do?
See issue for details

### What issues does this PR fix or reference?
Fixes: #63231

### Previous Behavior
Long run times for `file.tidied` if a given directory during the walk had _a lot_ of subdirectories.

### New Behavior
Mo fasta

### Merge requirements satisfied?
**[NOTICE] Bug fixes or features added to Salt require tests.**
<!-- Please review the [test documentation](https://docs.saltproject.io/en/master/topics/tutorials/writing_tests.html) for details on how to implement tests into Salt's test suite. -->
- [x] Docs
- [x] Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
- [x] Tests written/updated

### Commits signed with GPG?
Yes

Please review [Salt's Contributing Guide](https://docs.saltproject.io/en/master/topics/development/contributing.html) for best practices.

See GitHub's [page on GPG signing](https://help.github.com/articles/signing-commits-using-gpg/) for more information about signing commits with GPG.


Problem statement:
[BUG] serious performance issues with the file.tidied module
**Description**
We ran into some serious performance issues with the file.tidied module. We have a temp directory with 500k files under it. A pure python `os.walk` with a `stat` of each file + dir takes 2.3s. However salt runs on that host are taking almost an hour with tons of CPU time spent by Salt. `strace` shows it doing about 3 `stat`s per file and only progressing about 1 file per every 10ms.

**Setup**

_tidy.sls_:

```yaml
tidy_temp:
  file.tidied:
    - name: /tmp
    - rmdirs: false
    - rmlinks: false
    - full_path_match: true
    - time_comparison: mtime
    - age_size_only: age
    - exclude:
      - .*/crash/.*
    - age: 30
```

**Steps to Reproduce the behavior**
Run the `file.tidied` state from the current master branch on a directory with over 500k files in it.

**Expected behavior**
I'd expect the time for the code to run to be much closer to a raw `os.walk` + `os.stat` test.

**Versions Report**
<details><summary>salt --versions-report</summary>
(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

```yaml
Salt Version:
          Salt: 3006.0+0na.5b18e86
 
Dependency Versions:
          cffi: Not Installed
      cherrypy: Not Installed
      dateutil: Not Installed
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.2
       libgit2: Not Installed
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.4
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: Not Installed
      pycrypto: 2.6.1
  pycryptodome: 3.16.0
        pygit2: Not Installed
        Python: 3.7.3 (default, Oct 31 2022, 14:04:00)
  python-gnupg: Not Installed
        PyYAML: 6.0
         PyZMQ: 24.0.1
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.4
 
System Versions:
          dist: debian 10 buster
        locale: UTF-8
       machine: x86_64
       release: 4.19.0-22-amd64
        system: Linux
       version: Debian GNU/Linux 10 buster
```
</details>

Hints from issue discussion:
Ended up tracking the issue down to this line:

https://github.com/saltstack/salt/blob/fb1b562104db6944eabeb83b828992a72adab51d/salt/states/file.py#L2155

One directory had 61k directories in it, so performing a "is this string in this list" check was very costly. PR incoming.

Commit message: fix test for new file.tidied operation

Patch to review:
diff --git a/changelog/63231.fixed b/changelog/63231.fixed
new file mode 100644
index 000000000000..cda743d7bc4a
--- /dev/null
+++ b/changelog/63231.fixed
@@ -0,0 +1 @@
+Fix serious performance issues with the file.tidied module
diff --git a/salt/states/file.py b/salt/states/file.py
index e76a7cdd97e7..acab469edb0c 100644
--- a/salt/states/file.py
+++ b/salt/states/file.py
@@ -286,6 +286,7 @@ def run():
 import posixpath
 import re
 import shutil
+import stat
 import sys
 import time
 import traceback
@@ -2073,15 +2074,18 @@ def tidied(
 
     ret = {"name": name, "changes": {}, "result": True, "comment": ""}
 
-    if age_size_logical_operator.upper() not in ["AND", "OR"]:
+    age_size_logical_operator = age_size_logical_operator.upper()
+    if age_size_logical_operator not in ["AND", "OR"]:
         age_size_logical_operator = "OR"
         log.warning("Logical operator must be 'AND' or 'OR'. Defaulting to 'OR'...")
 
-    if age_size_only and age_size_only.lower() not in ["age", "size"]:
-        age_size_only = None
-        log.warning(
-            "age_size_only parameter must be 'age' or 'size' if set. Defaulting to 'None'..."
-        )
+    if age_size_only:
+        age_size_only = age_size_only.lower()
+        if age_size_only not in ["age", "size"]:
+            age_size_only = None
+            log.warning(
+                "age_size_only parameter must be 'age' or 'size' if set. Defaulting to 'None'..."
+            )
 
     # Check preconditions
     if not os.path.isabs(name):
@@ -2134,44 +2138,41 @@ def _matches(name):
             path = os.path.join(root, elem)
             try:
                 if os.path.islink(path):
-                    # Get timestamp of symlink (not symlinked file)
-                    if time_comparison == "ctime":
-                        mytimestamp = os.lstat(path).st_ctime
-                    elif time_comparison == "mtime":
-                        mytimestamp = os.lstat(path).st_mtime
-                    else:
-                        mytimestamp = os.lstat(path).st_atime
                     if not rmlinks:
                         deleteme = False
+                    # Get info of symlink (not symlinked file)
+                    mystat = os.lstat(path)
                 else:
-                    # Get timestamp of file or directory
-                    if time_comparison == "ctime":
-                        mytimestamp = os.path.getctime(path)
-                    elif time_comparison == "mtime":
-                        mytimestamp = os.path.getmtime(path)
-                    else:
-                        mytimestamp = os.path.getatime(path)
+                    mystat = os.stat(path)
 
-                    if elem in dirs:
+                    if stat.S_ISDIR(mystat.st_mode):
                         # Check if directories should be deleted at all
                         deleteme = rmdirs
                     else:
                         # Get size of regular file
-                        mysize = os.path.getsize(path)
+                        mysize = mystat.st_size
+
+                if time_comparison == "ctime":
+                    mytimestamp = mystat.st_ctime
+                elif time_comparison == "mtime":
+                    mytimestamp = mystat.st_mtime
+                else:
+                    mytimestamp = mystat.st_atime
 
                 # Calculate the age and set the name to match
                 myage = abs(today - date.fromtimestamp(mytimestamp))
+
                 filename = elem
                 if full_path_match:
                     filename = path
 
                 # Verify against given criteria, collect all elements that should be removed
-                if age_size_only and age_size_only.lower() in ["age", "size"]:
-                    if age_size_only.lower() == "age":
+                if age_size_only and age_size_only in ["age", "size"]:
+                    if age_size_only == "age":
                         compare_age_size = myage.days >= age
                     else:
                         compare_age_size = mysize >= size
-                elif age_size_logical_operator.upper() == "AND":
+                elif age_size_logical_operator == "AND":
                     compare_age_size = mysize >= size and myage.days >= age
                 else:
                     compare_age_size = mysize >= size or myage.days >= age
@@ -2180,6 +2181,8 @@ def _matches(name):
                     todelete.append(path)
             except FileNotFoundError:
                 continue
+            except PermissionError:
+                log.warning("Unable to read %s due to permissions error.", path)
 
     # Now delete the stuff
     if todelete:
@@ -2192,7 +2195,7 @@ def _matches(name):
         # Iterate over collected items
         try:
             for path in todelete:
-                __salt__["file.remove"](path, force=True)
+                __salt__["file.remove"](path)
                 ret["changes"]["removed"].append(path)
         except CommandExecutionError as exc:
             return _error(ret, "{}".format(exc))
diff --git a/tests/pytests/unit/states/file/test_tidied.py b/tests/pytests/unit/states/file/test_tidied.py
index c1873a273203..e31e33293e61 100644
--- a/tests/pytests/unit/states/file/test_tidied.py
+++ b/tests/pytests/unit/states/file/test_tidied.py
@@ -1,7 +1,6 @@
 import logging
 import os
 from datetime import datetime, timedelta
-from types import SimpleNamespace
 
 import pytest
 
@@ -11,7 +10,7 @@
 import salt.utils.platform
 import salt.utils.win_functions
 import salt.utils.yaml
-from tests.support.mock import MagicMock, patch
+from tests.support.mock import MagicMock, PropertyMock, patch
 
 log = logging.getLogger(__name__)
 
@@ -33,11 +32,33 @@ def test__tidied():
     ]
     today_delta = datetime.today() - datetime.utcfromtimestamp(0)
     remove = MagicMock(name="file.remove")
+
+    mystat = MagicMock()
+    mystat.st_atime = today_delta.total_seconds()
+    # dir = 16877
+    # file = 33188
+    mock_st_mode = PropertyMock(
+        side_effect=[
+            33188,
+            33188,
+            16877,
+            33188,
+            16877,
+            16877,
+            33188,
+            33188,
+            16877,
+            33188,
+            16877,
+            16877,
+        ]
+    )
+    type(mystat).st_mode = mock_st_mode
+    mystat.st_size = 10
+
     with patch("os.walk", return_value=walker), patch(
         "os.path.islink", return_value=False
-    ), patch("os.path.getatime", return_value=today_delta.total_seconds()), patch(
-        "os.path.getsize", return_value=10
-    ), patch.dict(
+    ), patch("os.stat", return_value=mystat), patch.dict(
         filestate.__opts__, {"test": False}
     ), patch.dict(
         filestate.__salt__, {"file.remove": remove}
@@ -57,15 +78,15 @@ def test__tidied():
         "result": True,
         "comment": "Removed 3 files or directories from directory {}".format(name),
     }
-    assert exp == ret
+    assert ret == exp
     assert remove.call_count == 3
 
     remove.reset_mock()
+    mock_st_mode.reset_mock()
+
     with patch("os.walk", return_value=walker), patch(
         "os.path.islink", return_value=False
-    ), patch("os.path.getatime", return_value=today_delta.total_seconds()), patch(
-        "os.path.getsize", return_value=10
-    ), patch.dict(
+    ), patch("os.stat", return_value=mystat), patch.dict(
         filestate.__opts__, {"test": False}
     ), patch.dict(
         filestate.__salt__, {"file.remove": remove}
@@ -88,7 +109,7 @@ def test__tidied():
         "result": True,
         "comment": "Removed 6 files or directories from directory {}".format(name),
     }
-    assert exp == ret
+    assert ret == exp
     assert remove.call_count == 6
 
 
@@ -120,12 +141,41 @@ def test_tidied_with_exclude():
         ("test", ["test1", "test2"], ["file3"]),
     ]
     today_delta = datetime.today() - datetime.utcfromtimestamp(0)
+
+    mystat = MagicMock()
+    mystat.st_atime = today_delta.total_seconds()
+    # dir = 16877
+    # file = 33188
+    mock_st_mode = PropertyMock(
+        side_effect=[
+            33188,
+            33188,
+            16877,
+            33188,
+            16877,
+            16877,
+            33188,
+            33188,
+            16877,
+            33188,
+            16877,
+            16877,
+            33188,
+            33188,
+            16877,
+            33188,
+            16877,
+            16877,
+        ]
+    )
+    type(mystat).st_mode = mock_st_mode
+    mystat.st_size = 10
+
     remove = MagicMock(name="file.remove")
+
     with patch("os.walk", return_value=walker), patch(
         "os.path.islink", return_value=False
-    ), patch("os.path.getatime", return_value=today_delta.total_seconds()), patch(
-        "os.path.getsize", return_value=10
-    ), patch.dict(
+    ), patch("os.stat", return_value=mystat), patch.dict(
         filestate.__opts__, {"test": False}
     ), patch.dict(
         filestate.__salt__, {"file.remove": remove}
@@ -144,15 +194,13 @@ def test_tidied_with_exclude():
         "result": True,
         "comment": "Removed 2 files or directories from directory {}".format(name),
     }
-    assert exp == ret
+    assert ret == exp
     assert remove.call_count == 2
 
     remove.reset_mock()
     with patch("os.walk", return_value=walker), patch(
         "os.path.islink", return_value=False
-    ), patch("os.path.getatime", return_value=today_delta.total_seconds()), patch(
-        "os.path.getsize", return_value=10
-    ), patch.dict(
+    ), patch("os.stat", return_value=mystat), patch.dict(
         filestate.__opts__, {"test": False}
     ), patch.dict(
         filestate.__salt__, {"file.remove": remove}
@@ -174,15 +222,13 @@ def test_tidied_with_exclude():
         "result": True,
         "comment": "Removed 5 files or directories from directory {}".format(name),
     }
-    assert exp == ret
+    assert ret == exp
     assert remove.call_count == 5
 
     remove.reset_mock()
     with patch("os.walk", return_value=walker), patch(
         "os.path.islink", return_value=False
-    ), patch("os.path.getatime", return_value=today_delta.total_seconds()), patch(
-        "os.path.getsize", return_value=10
-    ), patch.dict(
+    ), patch("os.stat", return_value=mystat), patch.dict(
         filestate.__opts__, {"test": False}
     ), patch.dict(
         filestate.__salt__, {"file.remove": remove}
@@ -212,7 +258,7 @@ def test_tidied_with_exclude():
         "result": True,
         "comment": "Removed 6 files or directories from directory {}".format(name),
     }
-    assert exp == ret
+    assert ret == exp
     assert remove.call_count == 6
 
 
@@ -227,12 +273,41 @@ def test_tidied_with_full_path_exclude():
         ("test", ["test1", "test2"], ["file3"]),
     ]
     today_delta = datetime.today() - datetime.utcfromtimestamp(0)
+
+    mystat = MagicMock()
+    mystat.st_atime = today_delta.total_seconds()
+    # dir = 16877
+    # file = 33188
+    mock_st_mode = PropertyMock(
+        side_effect=[
+            33188,
+            33188,
+            16877,
+            33188,
+            16877,
+            16877,
+            33188,
+            33188,
+            16877,
+            33188,
+            16877,
+            16877,
+            33188,
+            33188,
+            16877,
+            33188,
+            16877,
+            16877,
+        ]
+    )
+    type(mystat).st_mode = mock_st_mode
+    mystat.st_size = 10
+
     remove = MagicMock(name="file.remove")
+
     with patch("os.walk", return_value=walker), patch(
         "os.path.islink", return_value=False
-    ), patch("os.path.getatime", return_value=today_delta.total_seconds()), patch(
-        "os.path.getsize", return_value=10
-    ), patch.dict(
+    ), patch("os.stat", return_value=mystat), patch.dict(
         filestate.__opts__, {"test": False}
     ), patch.dict(
         filestate.__salt__, {"file.remove": remove}
@@ -255,15 +330,13 @@ def test_tidied_with_full_path_exclude():
         "result": True,
         "comment": "Removed 2 files or directories from directory {}".format(name),
     }
-    assert exp == ret
+    assert ret == exp
     assert remove.call_count == 2
 
     remove.reset_mock()
     with patch("os.walk", return_value=walker), patch(
         "os.path.islink", return_value=False
-    ), patch("os.path.getatime", return_value=today_delta.total_seconds()), patch(
-        "os.path.getsize", return_value=10
-    ), patch.dict(
+    ), patch("os.stat", return_value=mystat), patch.dict(
         filestate.__opts__, {"test": False}
     ), patch.dict(
         filestate.__salt__, {"file.remove": remove}
@@ -290,15 +363,13 @@ def test_tidied_with_full_path_exclude():
         "result": True,
         "comment": "Removed 5 files or directories from directory {}".format(name),
     }
-    assert exp == ret
+    assert ret == exp
     assert remove.call_count == 5
 
     remove.reset_mock()
     with patch("os.walk", return_value=walker), patch(
         "os.path.islink", return_value=False
-    ), patch("os.path.getatime", return_value=today_delta.total_seconds()), patch(
-        "os.path.getsize", return_value=10
-    ), patch.dict(
+    ), patch("os.stat", return_value=mystat), patch.dict(
         filestate.__opts__, {"test": False}
     ), patch.dict(
         filestate.__salt__, {"file.remove": remove}
@@ -326,7 +397,7 @@ def test_tidied_with_full_path_exclude():
         "result": True,
         "comment": "Removed 6 files or directories from directory {}".format(name),
     }
-    assert exp == ret
+    assert ret == exp
     assert remove.call_count == 6
 
 
@@ -382,12 +453,29 @@ def test_tidied_age_size_args_AND_operator_age_not_size_age_only():
         ("test", ["test1", "test2"], ["file3"]),
     ]
     today_delta = (datetime.today() - timedelta(days=14)) - datetime.utcfromtimestamp(0)
+
+    mystat = MagicMock()
+    mystat.st_atime = today_delta.total_seconds()
+    # dir = 16877
+    # file = 33188
+    mock_st_mode = PropertyMock(
+        side_effect=[
+            33188,
+            33188,
+            16877,
+            33188,
+            16877,
+            16877,
+        ]
+    )
+    type(mystat).st_mode = mock_st_mode
+    mystat.st_size = 10
+
     remove = MagicMock(name="file.remove")
+
     with patch("os.walk", return_value=walker), patch(
         "os.path.islink", return_value=False
-    ), patch("os.path.getatime", return_value=today_delta.total_seconds()), patch(
-        "os.path.getsize", return_value=10
-    ), patch.dict(
+    ), patch("os.stat", return_value=mystat), patch.dict(
         filestate.__opts__, {"test": False}
     ), patch.dict(
         filestate.__salt__, {"file.remove": remove}
@@ -470,12 +558,29 @@ def test_tidied_age_size_args_AND_operator_size_not_age_size_only():
         ("test", ["test1", "test2"], ["file3"]),
     ]
     today_delta = (datetime.today() - timedelta(days=14)) - datetime.utcfromtimestamp(0)
+
+    mystat = MagicMock()
+    mystat.st_atime = today_delta.total_seconds()
+    # dir = 16877
+    # file = 33188
+    mock_st_mode = PropertyMock(
+        side_effect=[
+            33188,
+            33188,
+            16877,
+            33188,
+            16877,
+            16877,
+        ]
+    )
+    type(mystat).st_mode = mock_st_mode
+    mystat.st_size = 10
+
     remove = MagicMock(name="file.remove")
+
     with patch("os.walk", return_value=walker), patch(
         "os.path.islink", return_value=False
-    ), patch("os.path.getatime", return_value=today_delta.total_seconds()), patch(
-        "os.path.getsize", return_value=10
-    ), patch.dict(
+    ), patch("os.stat", return_value=mystat), patch.dict(
         filestate.__opts__, {"test": False}
     ), patch.dict(
         filestate.__salt__, {"file.remove": remove}
@@ -517,12 +622,29 @@ def test_tidied_age_size_args_AND_operator_size_and_age():
         ("test", ["test1", "test2"], ["file3"]),
     ]
     today_delta = (datetime.today() - timedelta(days=14)) - datetime.utcfromtimestamp(0)
+
+    mystat = MagicMock()
+    mystat.st_atime = today_delta.total_seconds()
+    # dir = 16877
+    # file = 33188
+    mock_st_mode = PropertyMock(
+        side_effect=[
+            33188,
+            33188,
+            16877,
+            33188,
+            16877,
+            16877,
+        ]
+    )
+    type(mystat).st_mode = mock_st_mode
+    mystat.st_size = 10
+
     remove = MagicMock(name="file.remove")
+
     with patch("os.walk", return_value=walker), patch(
         "os.path.islink", return_value=False
-    ), patch("os.path.getatime", return_value=today_delta.total_seconds()), patch(
-        "os.path.getsize", return_value=10
-    ), patch.dict(
+    ), patch("os.stat", return_value=mystat), patch.dict(
         filestate.__opts__, {"test": False}
     ), patch.dict(
         filestate.__salt__, {"file.remove": remove}
@@ -592,14 +714,34 @@ def test_tidied_rmlinks():
         ("test", ["test1", "test2"], ["file3"]),
     ]
     today_delta = (datetime.today() - timedelta(days=14)) - datetime.utcfromtimestamp(0)
-    mock_lstat = MagicMock(return_value=SimpleNamespace(st_atime=today_delta))
+
+    mystat = MagicMock()
+    mystat.st_atime = today_delta.total_seconds()
+    # dir = 16877
+    # file = 33188
+    mock_st_mode = PropertyMock(
+        side_effect=[
+            33188,
+            16877,
+            33188,
+            16877,
+            16877,
+        ]
+    )
+    type(mystat).st_mode = mock_st_mode
+    mystat.st_size = 10
+
+    mylstat = MagicMock()
+    mylstat.st_atime = today_delta.total_seconds()
+    mylstat.st_mode = 33188
+    mylstat.st_size = 10
+
     remove = MagicMock(name="file.remove")
+
     with patch("os.walk", return_value=walker), patch(
         "os.path.islink", side_effect=[False, True, False, False, False, False]
-    ), patch("os.path.getatime", return_value=today_delta.total_seconds()), patch(
-        "os.lstat", return_value=mock_lstat
-    ), patch(
-        "os.path.getsize", return_value=10
+    ), patch("os.lstat", return_value=mylstat), patch(
+        "os.stat", return_value=mystat
     ), patch.dict(
         filestate.__opts__, {"test": False}
     ), patch.dict(
diff --git a/tests/pytests/unit/states/test_file.py b/tests/pytests/unit/states/test_file.py
index 1cb1593000ce..fba4cff883fc 100644
--- a/tests/pytests/unit/states/test_file.py
+++ b/tests/pytests/unit/states/test_file.py
@@ -32,7 +32,7 @@ def test_file_absent_should_use_force_mode_for_file_remove(fake_remove, mock_mod
 
 
 # TODO: This file.matches test should be a functional test instead. For now this is probably good enough -W. Werner, 2020-09-15
-def test_file_tidied_should_use_force_mode_for_file_remove(fake_remove):
+def test_file_tidied_for_file_remove(fake_remove):
     patch_is_dir = patch("os.path.isdir", autospec=True, return_value=True)
     patch_os_walk = patch(
         "os.walk",
@@ -42,15 +42,14 @@ def test_file_tidied_should_use_force_mode_for_file_remove(fake_remove):
     patch_stat = patch("os.stat", autospec=True)
     with patch_os_walk, patch_is_dir, patch_stat as fake_stat:
         fake_stat.return_value.st_atime = 1600356711.1166897
+        fake_stat.return_value.st_mode = 33188
         fake_stat.return_value.st_size = 9001  # It's over 9000!
 
         file.tidied("/some/directory/tree")
 
     call_root_file1 = "some root{}file1".format(os.sep)
     call_root_file2 = "some root{}file2".format(os.sep)
-    fake_remove.assert_has_calls(
-        [call(call_root_file1, force=True), call(call_root_file2, force=True)]
-    )
+    fake_remove.assert_has_calls([call(call_root_file1), call(call_root_file2)])
 
 
 # TODO: This file.copy test should be a functional test instead. For now this is probably good enough -W. Werner, 2020-09-15


Code review comments:
diff_hunk: @@ -2134,44 +2138,41 @@ def _matches(name):
             path = os.path.join(root, elem)
             try:
                 if os.path.islink(path):
-                    # Get timestamp of symlink (not symlinked file)
-                    if time_comparison == "ctime":
-                        mytimestamp = os.lstat(path).st_ctime
-                    elif time_comparison == "mtime":
-                        mytimestamp = os.lstat(path).st_mtime
-                    else:
-                        mytimestamp = os.lstat(path).st_atime
                     if not rmlinks:
                         deleteme = False
+                    # Get info of symlink (not symlinked file)
+                    mystat = os.lstat(path)
                 else:
-                    # Get timestamp of file or directory
-                    if time_comparison == "ctime":
-                        mytimestamp = os.path.getctime(path)
-                    elif time_comparison == "mtime":
-                        mytimestamp = os.path.getmtime(path)
-                    else:
-                        mytimestamp = os.path.getatime(path)
+                    mystat = os.stat(path)
 
-                    if elem in dirs:
+                    if stat.S_ISDIR(mystat.st_mode):
                         # Check if directories should be deleted at all
                         deleteme = rmdirs
                     else:
                         # Get size of regular file
-                        mysize = os.path.getsize(path)
+                        mysize = mystat.st_size
+
+                if time_comparison == "ctime":
+                    mytimestamp = mystat.st_ctime
+                elif time_comparison == "mtime":
+                    mytimestamp = mystat.st_mtime
+                else:
+                    mytimestamp = mystat.st_atime
 
                 # Calculate the age and set the name to match
                 myage = abs(today - date.fromtimestamp(mytimestamp))
+
                 filename = elem
                 if full_path_match:
                     filename = path
 
                 # Verify against given criteria, collect all elements that should be removed
-                if age_size_only and age_size_only.lower() in ["age", "size"]:
-                    if age_size_only.lower() == "age":
+                if age_size_only and age_size_only in ["age", "size"]:
path: salt/states/file.py
text: @user1:
**suggestion, non-blocking*** FWIW if your goal is squeezing the fastest times out of this:

```
import timeit


print(timeit.timeit('"foo" in ("AND", "OR")'))
print(timeit.timeit('"foo" in {"AND", "OR"}'))
print(timeit.timeit('"foo" in ["AND", "OR"]'))
```

These results suggest that a set literal might be faster here.

Probably not worth it in other places, but it looks like it could be of some help here.