perf: optimize WorkspaceService::getFile#5959
Conversation
Signed-off-by: Robin Appelman <robin@icewind.nl>
|
What do you think about a backport? I'm aware it's a performance optimization in the first place, but removing nodeExists is also bugfix. The stack trace below is from 29.0.7 with a faulty external dav storage. {
"Exception": "OCP\\Files\\StorageNotAvailableException",
"Message": "Sabre\\HTTP\\ClientHttpException: Internal Server Error",
"Code": 1,
"Trace": [
{
"file": "lib/private/Files/Storage/DAV.php",
"line": 298,
"function": "convertException",
"class": "OC\\Files\\Storage\\DAV",
"type": "->"
},
{
"file": "lib/private/Files/Storage/DAV.php",
"line": 600,
"function": "propfind",
"class": "OC\\Files\\Storage\\DAV",
"type": "->"
},
{
"file": "lib/private/Files/Storage/DAV.php",
"line": 660,
"function": "getMetaData",
"class": "OC\\Files\\Storage\\DAV",
"type": "->"
},
{
"file": "lib/private/Files/Storage/Common.php",
"line": 470,
"function": "stat",
"class": "OC\\Files\\Storage\\DAV",
"type": "->"
},
{
"file": "apps/files_sharing/lib/External/Storage.php",
"line": 213,
"function": "test",
"class": "OC\\Files\\Storage\\Common",
"type": "->"
},
{
"file": "lib/private/Files/Storage/Wrapper/Wrapper.php",
"line": 480,
"function": "test",
"class": "OCA\\Files_Sharing\\External\\Storage",
"type": "->"
},
{
"file": "lib/private/Files/Storage/Wrapper/Availability.php",
"line": 69,
"function": "test",
"class": "OC\\Files\\Storage\\Wrapper\\Wrapper",
"type": "->"
},
{
"file": "lib/private/Files/Storage/Wrapper/Availability.php",
"line": 83,
"function": "updateAvailability",
"class": "OC\\Files\\Storage\\Wrapper\\Availability",
"type": "->",
"args": [
"*** sensitive parameters replaced ***"
]
},
{
"file": "lib/private/Files/Storage/Wrapper/Availability.php",
"line": 92,
"function": "isAvailable",
"class": "OC\\Files\\Storage\\Wrapper\\Availability",
"type": "->"
},
{
"file": "lib/private/Files/Storage/Wrapper/Availability.php",
"line": 242,
"function": "checkAvailability",
"class": "OC\\Files\\Storage\\Wrapper\\Availability",
"type": "->"
},
{
"file": "lib/private/Files/Storage/Wrapper/Wrapper.php",
"line": 233,
"function": "file_exists",
"class": "OC\\Files\\Storage\\Wrapper\\Availability",
"type": "->"
},
{
"file": "lib/private/Files/View.php",
"line": 1342,
"function": "file_exists",
"class": "OC\\Files\\Storage\\Wrapper\\Wrapper",
"type": "->"
},
{
"file": "lib/private/Files/View.php",
"line": 1383,
"function": "getCacheEntry",
"class": "OC\\Files\\View",
"type": "->"
},
{
"file": "lib/private/Files/Node/Root.php",
"line": 208,
"function": "getFileInfo",
"class": "OC\\Files\\View",
"type": "->"
},
{
"file": "lib/private/Files/Node/Folder.php",
"line": 139,
"function": "get",
"class": "OC\\Files\\Node\\Root",
"type": "->"
},
{
"file": "lib/private/Files/Node/Folder.php",
"line": 148,
"function": "get",
"class": "OC\\Files\\Node\\Folder",
"type": "->"
},
{
"file": "apps/text/lib/Service/WorkspaceService.php",
"line": 27,
"function": "nodeExists",
"class": "OC\\Files\\Node\\Folder",
"type": "->"
},
{
"file": "apps/text/lib/DAV/WorkspacePlugin.php",
"line": 119,
"function": "getFile",
"class": "OCA\\Text\\Service\\WorkspaceService",
"type": "->"
},
{
"file": "3rdparty/sabre/event/lib/WildcardEmitterTrait.php",
"line": 89,
"function": "propFind",
"class": "OCA\\Text\\DAV\\WorkspacePlugin",
"type": "->"
},
{
"file": "3rdparty/sabre/dav/lib/DAV/Server.php",
"line": 1052,
"function": "emit",
"class": "Sabre\\DAV\\Server",
"type": "->"
},
{
"file": "3rdparty/sabre/dav/lib/DAV/Server.php",
"line": 984,
"function": "getPropertiesByNode",
"class": "Sabre\\DAV\\Server",
"type": "->"
},
{
"file": "3rdparty/sabre/dav/lib/DAV/Server.php",
"line": 1662,
"function": "getPropertiesIteratorForPath",
"class": "Sabre\\DAV\\Server",
"type": "->"
},
{
"file": "3rdparty/sabre/dav/lib/DAV/Server.php",
"line": 1647,
"function": "writeMultiStatus",
"class": "Sabre\\DAV\\Server",
"type": "->"
},
{
"file": "3rdparty/sabre/dav/lib/DAV/CorePlugin.php",
"line": 346,
"function": "generateMultiStatus",
"class": "Sabre\\DAV\\Server",
"type": "->"
},
{
"file": "3rdparty/sabre/event/lib/WildcardEmitterTrait.php",
"line": 89,
"function": "httpPropFind",
"class": "Sabre\\DAV\\CorePlugin",
"type": "->"
},
{
"file": "3rdparty/sabre/dav/lib/DAV/Server.php",
"line": 472,
"function": "emit",
"class": "Sabre\\DAV\\Server",
"type": "->"
},
{
"file": "3rdparty/sabre/dav/lib/DAV/Server.php",
"line": 253,
"function": "invokeMethod",
"class": "Sabre\\DAV\\Server",
"type": "->"
},
{
"file": "3rdparty/sabre/dav/lib/DAV/Server.php",
"line": 321,
"function": "start",
"class": "Sabre\\DAV\\Server",
"type": "->"
},
{
"file": "apps/dav/lib/Server.php",
"line": 383,
"function": "exec",
"class": "Sabre\\DAV\\Server",
"type": "->"
},
{
"file": "apps/dav/appinfo/v2/remote.php",
"line": 35,
"function": "exec",
"class": "OCA\\DAV\\Server",
"type": "->"
},
{
"file": "remote.php",
"line": 172,
"args": [
"apps/dav/appinfo/v2/remote.php"
],
"function": "require_once"
}
],
"File": "lib/private/Files/Storage/DAV.php",
"Line": 889,
"Hint": "Storage is temporarily not available",
"message": "External storage not available: Sabre\\HTTP\\ClientHttpException: Internal Server Error",
"exception": {},
"CustomMessage": "External storage not available: Sabre\\HTTP\\ClientHttpException: Internal Server Error"
} |
|
/backport to stable29 |
|
The backport to # Switch to the target branch and update it
git checkout stable29
git pull origin stable29
# Create the new backport branch
git checkout -b backport/5959/stable29
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 5020d9b0
# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/5959/stable29Error: Failed to push branch backport/5959/stable29: fatal: could not read Username for 'https://github.com': No such device or address Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports. |
Agreed. Manual backport as backportbot failed for some reason: #6461 |
📝 Summary
Optimize
WorkspaceService::getFileby removing the separate check fornodeExists🏁 Checklist
npm run lint/npm run stylelint/composer run cs:check)