Summary: | [GTK][WK2] Simplify ProcessExecutablePathGtk | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zan Dobersek <zan> | ||||||||
Component: | New Bugs | Assignee: | Zan Dobersek <zan> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | betravis, cgarcia, commit-queue, esprehn+autocc, glenn, gyuyoung.kim, macpherson, menard, mrobinson | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Zan Dobersek
2013-12-23 12:03:22 PST
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. |