[GTK][WK2] Simplify ProcessExecutablePathGtk
Created attachment 219923 [details] Patch
Comment on attachment 219923 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=219923&action=review > Source/WebKit2/Shared/gtk/ProcessExecutablePathGtk.cpp:51 > + static CString executablePath = getCurrentExecutablePath(); > if (!executablePath.isNull()) { > - String processPath = pathByAppendingComponent(executablePath, processName); > + String processPath = pathByAppendingComponent(directoryName(filenameToString(executablePath.data())), processName); > if (fileExists(processPath)) > return processPath; This is a little slower, because directoryName and filenameToString runs for each path, but not much.
Comment on attachment 219923 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=219923&action=review >> Source/WebKit2/Shared/gtk/ProcessExecutablePathGtk.cpp:51 >> return processPath; > > This is a little slower, because directoryName and filenameToString runs for each path, but not much. I can further store return values of findWebKitProcess in static variables in executablePathOf*Process. This would limit the number of times that findWebKitProcess is called to 3. If that sounds OK I can upload a new patch.
(In reply to comment #3) > (From update of attachment 219923 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=219923&action=review > > >> Source/WebKit2/Shared/gtk/ProcessExecutablePathGtk.cpp:51 > >> return processPath; > > > > This is a little slower, because directoryName and filenameToString runs for each path, but not much. > > I can further store return values of findWebKitProcess in static variables in executablePathOf*Process. This would limit the number of times that findWebKitProcess is called to 3. > > If that sounds OK I can upload a new patch. It seems simpler to cache the return value of directoryName(filenameToString(...)), but if this function is called only a few times, I don't think it's a big deal.
Comment on attachment 219923 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=219923&action=review >>>> Source/WebKit2/Shared/gtk/ProcessExecutablePathGtk.cpp:51 >>>> return processPath; >>> >>> This is a little slower, because directoryName and filenameToString runs for each path, but not much. >> >> I can further store return values of findWebKitProcess in static variables in executablePathOf*Process. This would limit the number of times that findWebKitProcess is called to 3. >> >> If that sounds OK I can upload a new patch. > > It seems simpler to cache the return value of directoryName(filenameToString(...)), but if this function is called only a few times, I don't think it's a big deal. This will now be called whenever a new process is being created. It's unnecessary work, really. I can offload all of this (getCurrentExecutablePath, filenameToString and directoryName calls) into a small helper static function. Its return value would then be stored in the static variable, so it would only get called once.
Created attachment 219929 [details] Patch
Comment on attachment 219929 [details] Patch Clearing flags on attachment: 219929 Committed r161020: <http://trac.webkit.org/changeset/161020>
All reviewed patches have been landed. Closing bug.
Reopening to attach new patch.
Created attachment 220760 [details] Initial patch
(In reply to comment #10) > Created an attachment (id=220760) [details] > Initial patch Wrong bug?
Yes, apologies. Was wondering where webkit-patch sent this one.