Bug Hunting Session
Bug 98705 - Reduce starting time on Windows by removing GetCaseCorrectPathName usage
Summary: Reduce starting time on Windows by removing GetCaseCorrectPathName usage
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: framework (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All Windows (All)
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords: difficultyBeginner, easyHack, skillCpp, topicDebug
Depends on:
Blocks:
 
Reported: 2016-03-16 12:22 UTC by László Németh
Modified: 2019-02-05 08:56 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments
Patch for resolving the Bug (5.06 KB, patch)
2017-03-24 10:36 UTC, aasthaagrrawal
Details
Patch for resolving the Bug (3.91 KB, patch)
2017-03-25 17:21 UTC, aasthaagrrawal
Details

Note You need to log in before you can comment on or make changes to this bug.
Description László Németh 2016-03-16 12:22:44 UTC
GetCaseCorrectPathName is a slow and likely obsolete function, mostly removed from the code.
Removing temporarily its last occurance (by the following test patch) resulted ~10% speed up in LO starting time on Windows.
So it would be fine to remove (or replace it GetLongPathName or something relevant, if needed) finally.

--- a/sal/osl/w32/file_dirvol.cxx
+++ b/sal/osl/w32/file_dirvol.cxx
@@ -1666,7 +1666,7 @@ oslFileError SAL_CALL osl_getFileStatus(
 
     if ( uFieldMask & osl_FileStatus_Mask_FileURL )
     {
-        if ( !pItemImpl->bFullPathNormalized )
+        if ( 0 && !pItemImpl->bFullPathNormalized )
         {
             ::osl::LongPathBuffer< sal_Unicode > aBuffer( MAX_LONG_PATH );
             sal_uInt32 nNewLen = GetCaseCorrectPathName( rtl_uString_getStr( pItemImpl->m_pFullPath ),

Some information in sal/osl/w32/file_url.cxx:

// Same as GetLongPathName but also 95/NT4
static DWORD GetCaseCorrectPathNameEx(
    LPWSTR  lpszPath,   // path buffer to convert
    DWORD   cchBuffer,      // size of path buffer
    DWORD   nSkipLevels,
    BOOL bCheckExistence )
Comment 1 jani 2016-03-17 07:33:27 UTC
Thanks for the solution, but please follow our guidelines for submitting patches:
https://wiki.documentfoundation.org/Development/GetInvolved

We use gerrit to review and apply patches.
Comment 2 Michael Meeks 2016-03-17 12:56:33 UTC
Jan - this is an easy-hack; the patch is just to highlight the win possible from optimizing the implementation here =) This is fundamentally a cleanup to use a built-in windows function instead of a hacked-up compatibility implementation.
Comment 3 aasthaagrrawal 2017-03-23 15:44:09 UTC
Hi, I am working on this bug.
Comment 4 aasthaagrrawal 2017-03-24 10:36:14 UTC
Created attachment 132121 [details]
Patch for resolving the Bug
Comment 5 Michael Meeks 2017-03-24 11:07:44 UTC
Thanks for the patch ! =) a few comments:

* can you submit it to gerrit ? - it takes a bit of setup - but good to get that done.

* can you remove the dead code rather than commenting it out ? if we had every line of code we'd removed over time still in the source - we would have 50x as much code and it would be un-readable & un-grep-able =)

Otherwise - this looks good; can you test your binaries on Windows XP ? and ensure that this method is triggered & continues to work ? I hope you can just copy your 'instdir' across and run it there.

Thanks !
Comment 6 aasthaagrrawal 2017-03-25 17:21:52 UTC
Created attachment 132143 [details]
Patch for resolving the Bug
Comment 7 Shinnok 2017-08-25 08:31:01 UTC
A polite ping, still working on this patch?
Comment 8 Shinnok 2017-08-28 12:05:50 UTC Comment hidden (obsolete)
Comment 9 Archit Jugran 2017-12-28 18:27:25 UTC
i Want to solve this bug as my first easy hack . i want to confirm whether i should simply remove all pieces of code where i find the function GetCaseCorrectPathName() called, or something more? .Thank you for your help in advance :)
Comment 10 Xisco Faulí 2018-01-24 13:48:52 UTC Comment hidden (obsolete)
Comment 11 Tor Lillqvist 2018-01-24 13:52:50 UTC
The "hack" to be done here is so trivial (especially when there already is a patch attached (that was ignored)), that any help would essentially mean doing the hack for him/her.
Comment 12 Shubham Verma 2018-02-25 20:12:54 UTC
Although the patch is there in the comments,written by someone else, but it has not been committed yet. I want to seek the permission to look into it for possible changes and correction if any error arises.
Comment 13 Michael Meeks 2018-02-26 11:26:26 UTC
Please do re-do the work yourself, and submit it to gerrit for testing if Archit  cannot do this =) Thanks !
Comment 14 Xisco Faulí 2018-03-30 02:32:16 UTC Comment hidden (obsolete)
Comment 15 Shubham Verma 2018-04-01 14:25:10 UTC
Sorry for the delay , I am still working on it.
Comment 16 Xisco Faulí 2018-05-02 02:31:15 UTC Comment hidden (obsolete)
Comment 17 Xisco Faulí 2018-06-02 03:07:12 UTC Comment hidden (obsolete)
Comment 18 Xisco Faulí 2018-07-03 02:36:52 UTC Comment hidden (obsolete)
Comment 19 Xisco Faulí 2018-08-03 02:49:44 UTC Comment hidden (obsolete)
Comment 20 Xisco Faulí 2018-09-03 02:40:38 UTC Comment hidden (obsolete)
Comment 21 Xisco Faulí 2018-10-04 02:53:17 UTC Comment hidden (obsolete)
Comment 22 Xisco Faulí 2018-11-04 03:55:26 UTC Comment hidden (obsolete)
Comment 23 Xisco Faulí 2018-12-05 03:47:06 UTC Comment hidden (obsolete)
Comment 24 Xisco Faulí 2019-01-05 03:39:55 UTC Comment hidden (obsolete)
Comment 25 Xisco Faulí 2019-02-05 03:44:19 UTC Comment hidden (obsolete)
Comment 26 Tor Lillqvist 2019-02-05 08:52:45 UTC Comment hidden (off-topic)