strip out non-trivial globals before main Background: Ordering and running constructors at startup consumes CPU time. Anything larger than a simple string should be switched to use the singleton pattern in 'sal' for this case. Any variable marked 'static' is potentially a problem. Skills: build, simple C++
Hi, I'd like to do this. Can you point me where exactly is the singleton pattern you use and which entry point (main) I should look at?
Look at the various StaticWhatever templates in sal/inc/rtl/instance.hxx . It is easy to find examples of their use throughout the code, but some really simple are in sal/rtl/source/alloc_fini.cxx .
Deteted "Easyhack" from summary
adding LibreOffice developer list as CC to unresolved EasyHacks for better visibility. see e.g. http://nabble.documentfoundation.org/minutes-of-ESC-call-td4076214.html for details
Hi ;) It's been a while. I'm starting to have free time again, so I've been poking around trying to find any globals that I can see as causing this issue. "Any variable marked 'static'" might not really be an issue save that they are global for only globals are constructed at startup but static locals are constructed when the declaration is first passed over.[1] [1] http://stackoverflow.com/questions/55510/when-do-function-level-static-variables-get-allocated-initialized
Let me add some more information to this Easy Hack, as it would be really quite useful to improve the situation :-) The simplest way to find global statics is like: git grep '^static[^(]*$' -- "*.cxx" and look after those that define a global variable of a more complex type (ie. skip the OUStrings / bools / ints for now). Then as the first thing, try to make it non-global if it is possible, like here: http://cgit.freedesktop.org/libreoffice/core/commit/?id=92bede3900e84d4f08efb81757ec95c518c7fa76 because that way this will get initialized when used for the first time. If that is not possible, or not practical, then you can do what is suggested in the initial comment. An example can be seen in this commit: http://cgit.freedesktop.org/libreoffice/core/commit/?id=020e29fb95d742b160feab1df6751e59d4108239 Hope that helps :-)
git grep '^static[^(]*=.*$' -- "*.cxx" | grep -v '\<\(bool\|sal_Bool\|OUString\|char\|int\|short\|long\|double\|sal_Int[0-9]*\|sal_uInt[0-9]*\|sal_Char\|sal_Unicode\)\>' | less filters out most of the trivial cases...
Another approach is to start soffice.bin under gdb and point a breakpoint on __do_global_ctors_aux (I think, something of that nature) and see what gets called during startup. Here are some other resources that might be useful http://neugierig.org/software/chromium/notes/2011/08/static-initializers.html http://stackoverflow.com/questions/335369/finding-c-static-initialization-order-problems
(In reply to comment #6) > Then as the first thing, try to make it non-global if it is possible, like > here: > > http://cgit.freedesktop.org/libreoffice/core/commit/ > ?id=92bede3900e84d4f08efb81757ec95c518c7fa76 > > because that way this will get initialized when used for the first time. If > that is not possible, or not practical, then you can do what is suggested in > the initial comment. An example can be seen in this commit: Note that initialization of local static variables is not thread-safe in C++03, so the above can only be used for cases where the local static variable ends up in code that is synchronized. Other cases need to use the rtl/instance.hxx idioms.
Hi, since this one isn't assigned to anyone, can I try to work on this bug?
(In reply to comment #10) > Hi, since this one isn't assigned to anyone, can I try to work on this bug? Sure you can.
Do I need to worry about synchronization between multiple threads when I try to create the singleton at the same time? I don't think I need to do the same for arrays right?
(In reply to comment #12) > Do I need to worry about synchronization between multiple threads when I try > to create the singleton at the same time? Yes, see comment 9. > I don't think I need to do the same for arrays right? The destinction wouldn't be so much between array vs. scalar, but whether or not it requires initialization at runtime.
Noel Grandin committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=705c48d32eec0aa5180e60ca157daca4b154e4a3 fdo#38835 strip out OUString globals It will be available in 4.4.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Noel Grandin committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=0375504f7be34d857859dfbaa312501e0eaaaad1 fdo#38835 strip out OString globals It will be available in 4.5.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Noel Grandin committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=86b44e8e78716fbfc1783d692debcbb201fb9bd4 fdo#38835 strip out OUString globals It will be available in 4.5.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Noel Grandin committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=2979ff295c9fafdb92cb56cd1f5ddb0a6b56cf20 fdo#38835 strip out OUString globals It will be available in 4.5.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Has patch been submitted for this problem or can i work around for it?
Hello everyone, this would be my first easyhack as well as first step in Open Source, I have builded LibreOffice, I would like to start contributing now.
Migrating Whiteboard tags to Keywords: (EasyHack DifficultyBeginner SkillCpp TopicCleanup ) [NinjaEdit]
Note that constants (currently either #defines or "static const ..." can -- with C++11 -- be turned into constexprs: http://en.cppreference.com/w/cpp/language/constexpr which should be generally preferable.
(In reply to Björn Michaelsen from comment #21) > Note that constants (currently either #defines or "static const ..." can -- > with C++11 -- be turned into constexprs: > > http://en.cppreference.com/w/cpp/language/constexpr > > which should be generally preferable. ...but isn't available in MSVC 2013 (and reportedly only partially in MSVC 2015, aka MSVC 14, <http://en.cppreference.com/w/cpp/compiler_support>). That's why SAL_CONSTEXPR (include/sal/types.h), and can only use it in ways that do not /require/ it to expand to "constexpr".
JanI is default CC for Easy Hacks (Add Jan; remove LibreOffice Dev List from CC) [NinjaEdit]
A polite ping, still working on this issue?
Rosen committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=b7dfa4d6625d066f5507420089434cb7ac29c968 tdf#38835 - strip out non-trivial globals before main It will be available in 5.3.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Is this easy hack still valid ?
A polite ping, still working on this bug?
Dear Gayan Thejawansha, This bug has been in ASSIGNED status for more than 3 months without any activity. Resetting it to NEW. Please assigned it back to yourself if you're still working on this.
Hi there, Since this bug wasn't assigned to anyone I would like to work on this. I read all the comments about the development of this bug. But some comments were hidden, so I would like know about the recent development.
(In reply to Abhishek from comment #46) > Hi there, Since this bug wasn't assigned to anyone I would like to work on > this. I read all the comments about the development of this bug. But some > comments were hidden, so I would like know about the recent development. Yes, please work on it. The status does not have to be changed, because multiple people can work on it at the same time. You can unhide comments by clicking on the [+] link, but there is nothing of value in them here. Do read all the visible comments, but note that we are able to use the vast majority of C++17 features at this point.
Hi there this is Hrithik Raj And i would like to fix this bug.I have assiged it to me. What to do next?
(In reply to Hrithik Raj from comment #48) > Hi there this is Hrithik Raj And i would like to fix this bug.I have assiged > it to me. > What to do next? Please don't change the assignee field. I have now had to change it back twice. See my comment 47. If you have questions, please be more specific. We assume you have read the description and looked at previous code changes related to this easy hack. Thus, "what to do next" is not an appropriate question to start with.
(In reply to Buovjaga from comment #49) > (In reply to Hrithik Raj from comment #48) > > Hi there this is Hrithik Raj And i would like to fix this bug.I have assiged > > it to me. > > What to do next? > > Please don't change the assignee field. I have now had to change it back > twice. See my comment 47. > > If you have questions, please be more specific. We assume you have read the > description and looked at previous code changes related to this easy hack. > Thus, "what to do next" is not an appropriate question to start with. Apologies. I am new to open source.Can you tell me where is the code where i have to look for this bug. I can see a directory in comment 2 .
(In reply to Hrithik Raj from comment #50) > Apologies. I am new to open source.Can you tell me where is the code where i > have to look for this bug. I can see a directory in comment 2 . Please read *all* of the comments and not just up to comment 2.
Quick question, is comment 9 still valid? That is, is LibreOffice still supposed to be compatible with C++03? I don't know what the minimum C++ version is across the various compilers. Is the C++ version documented somewhere? I see uses of unique_ptr and unordered_map, so I'm assuming at least C++11. And C++11 ensures function-local statics are initialized exactly once even if multiple threads call the function. Just checking before I make a change.
(In reply to ibarkleyyeung from comment #52) > Quick question, is comment 9 still valid? > > That is, is LibreOffice still supposed to be compatible with C++03? I don't > know what the minimum C++ version is across the various compilers. Is the > C++ version documented somewhere? > > I see uses of unique_ptr and unordered_map, so I'm assuming at least C++11. > And C++11 ensures function-local statics are initialized exactly once even > if multiple threads call the function. > > Just checking before I make a change. Please refer to the compiler baseline: https://git.libreoffice.org/core/+/master/README.md Cross-check it with https://en.cppreference.com/w/cpp/compiler_support LibreOffice can use the vast majority of features in C++17 and its standard library.
(In reply to Buovjaga from comment #53) > (In reply to ibarkleyyeung from comment #52) > > Quick question, is comment 9 still valid? > > > > That is, is LibreOffice still supposed to be compatible with C++03? I don't > > know what the minimum C++ version is across the various compilers. Is the > > C++ version documented somewhere? > > > > I see uses of unique_ptr and unordered_map, so I'm assuming at least C++11. > > And C++11 ensures function-local statics are initialized exactly once even > > if multiple threads call the function. > > > > Just checking before I make a change. > > Please refer to the compiler baseline: > https://git.libreoffice.org/core/+/master/README.md > Cross-check it with https://en.cppreference.com/w/cpp/compiler_support > LibreOffice can use the vast majority of features in C++17 and its standard > library. ...and comment 9 is indeed obsolete since <https://git.libreoffice.org/core/+/6517fd2107a5a71290afad8850da0eab51519bc6%5E!> "HAVE_THREADSAFE_STATICS sould always be true"
Ian Barkley-Yeung committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/4ee314959a377031a382b6ba42c6b5d5a0c0a367 tdf#38835: strip out non-trivial globals before main It will be available in 7.0.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Mesut Çifci committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/902e69d1a5473155915522d49f730720797a384f tdf#38835 strip out OUString globals It will be available in 7.0.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
This has been pretty well covered by now, I think we can close it
Mesut Çifci committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/34b3d1342579791d960d8bc77aa937ead49bd791 tdf38835 Avoid pointless globals It will be available in 7.0.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.