summaryrefslogtreecommitdiff
path: root/svl
AgeCommit message (Collapse)Author
2024-09-06tdf#161741 undo nits: NeedsClearRedo is private + prioritize TopLevelJustin Luth
moNeedsClearRedo is an implementation detail, so there should not be public functions for it. Plus, NeedsClearRedo can be called either against the CurrentLevel stack or the TopLevel stack (whatever that implies - there is no documentation...) and since it is a delayed call that means that multiple NeedsClearRedo attempts could be made, theoretically against both stacks. My initial implementation was "last one wins", (which may very well prove to be the best one) but whatever happens, it should be clearly intentional. Based on my limited skill at code reading, it sounds like CurrentLevel might be more of an implemention detail or a temporary expansion of a ListUndoAction, so I am guessing that any request for TopLevel clearing should be given priority. For Writer, it is always clearing the TopLevel stack sw/source/core/undo/docundo.cxx: ClearRedo() return SdrUndoManager::ImplClearRedo_NoLock(TopLevel); Change-Id: I195aefb696599f018712135a2e015549d534791f Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171984 Tested-by: Jenkins Reviewed-by: Miklos Vajna <vmiklos@collabora.com> Reviewed-by: Justin Luth <jluth@mail.com>
2024-09-04Fix signature hash on NSS backendEloi Montañés
When using the NSS backend, the SignedAttributes were added after the hash computation for the timestamp request, altering the final signature and rendering the timestamp unverifiable. This patch changes the order in which the attributes are added and uses a copy of the signer for the hash computation in order to generate a correct hash. Change-Id: I14cecf1819e80d7a2e470e0f1dee2d09bcbe852c Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171384 Tested-by: Jenkins Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
2024-08-31cid#1607947 silence Overflowed array index writeCaolán McNamara
Change-Id: I35b8db34e722e700df2771da99d76d42fe2cada6 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172640 Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com> Tested-by: Jenkins
2024-08-27tdf#158556 prevent lambda from allocating on heapNoel Grandin
bit of a hack Change-Id: Icf60fc1bc69e8c44e5612c05469164439f14a249 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172465 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2024-08-26tdf#158556 simplify StylePool APINoel Grandin
StylePool is only used inside writer, and only in very limited ways. Rather move the logic for the limits functions inside StylePool so I can then reduce the complexity of the iterator. Change-Id: I12bc5965b74abace28ae9190b35661a34f5005be Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172371 Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk> Tested-by: Jenkins
2024-08-23tdf#158556 make SfxGrabBagItem hashableNoel Grandin
Change-Id: I0af373e9c5c93a82eb37437ac365677700d45853 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172311 Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk> Tested-by: Jenkins
2024-08-16cid#1608509 silence Overflowed array index readCaolán McNamara
Change-Id: Ie42eb4de09968e5c21030936c693ad348c05bbdf Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171925 Tested-by: Caolán McNamara <caolan.mcnamara@collabora.com> Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
2024-08-16tdf#161741 tdf#161705 undo: delay ClearRedo until Undo finishesJustin Luth
I know NOTHING about the intricacies of Undo/Redo. However, this is my feeble attempt to add some sanity to it. It should not be the responsibility of the caller to know when they are allowed to call ClearRedo. This patch reverts commit 0cd000bb83719982c1fd2265ea040c82af5bf98e author Daniel Arato (NISZ) on Mon May 10 15:41:28 2021 +0200 tdf#141613 sw: avoid possible crash when undoing header creation which was not a sufficient hack. I hope this patch lays a better framework to handle future similar issues. vvv NAIVE OPTIMIST ALERT vvv mbDoing was added with commit 9cb53122e9e098bc8a6bf53c14b18415e369dd6d Author: Frank Schoenheit on Fri Oct 22 15:00:39 2010 +0200 undoapi: more I/SfxUndoManager changes in preparation of new Undo API features and has been untouched since then AFAICS, and there was never any mechanism to change mbDoing. In other words, it has been sitting there doing NOTHING. So, I am taking it over and using it how I imagine it was intended, and how it is documented. Change-Id: I7aa355cc6458ac8ba34ddb9ee73fc850dcbca702 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170793 Tested-by: Jenkins Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
2024-08-15SfxScriptOrganizerItem should not subclass SfxStringItemNoel Grandin
it never actually uses the superclass value. It has been this way since initial import. Change-Id: I99708c3ad8f1f2727ef87af56c62165d55f348d4 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171904 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2024-08-15make SfxStringItem hashableNoel Grandin
which shaves some time off loading complex files. Note that this class is often used as a superclass, so I checked all of the subclasses and marked some of them as "does not support hashing" until they can be independently verified to be safe Change-Id: Id4187eda8d6145e89e17dc10c2e3113b7a93da85 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171891 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2024-08-15merge CntUnencodedStringItem into SfxStringItemNoel Grandin
which simplifies the hierarhcy. We never allocate such a thing, we always allocate subclasses, and it has no real meaning by itself. Change-Id: Ie6b716c9ea6ca0efe0ae4f39ac345608c45534f4 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171890 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2024-08-12Simplify a bitMike Kaganski
Change-Id: I4079253afde4385eeb493cdb701ed735998365d1 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171768 Tested-by: Jenkins Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
2024-08-12cid#1608408 silence Overflowed integer argumentCaolán McNamara
Change-Id: I769e235a3fd8760c34c9b31b4b94b652ad74cde5 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171760 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
2024-08-08cid#1607356 COPY_INSTEAD_OF_MOVECaolán McNamara
and cid#1607117 COPY_INSTEAD_OF_MOVE cid#1607089 COPY_INSTEAD_OF_MOVE cid#1606977 COPY_INSTEAD_OF_MOVE cid#1606899 COPY_INSTEAD_OF_MOVE cid#1606785 COPY_INSTEAD_OF_MOVE cid#1606769 COPY_INSTEAD_OF_MOVE cid#1606740 COPY_INSTEAD_OF_MOVE cid#1606738 COPY_INSTEAD_OF_MOVE cid#1606675 COPY_INSTEAD_OF_MOVE cid#1606533 COPY_INSTEAD_OF_MOVE cid#1558100 COPY_INSTEAD_OF_MOVE cid#1558098 COPY_INSTEAD_OF_MOVE cid#1558083 COPY_INSTEAD_OF_MOVE cid#1558077 COPY_INSTEAD_OF_MOVE cid#1558074 COPY_INSTEAD_OF_MOVE Change-Id: Ica17dec2c2102ef85283fd883a0a4e64aec307c2 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171620 Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com> Tested-by: Jenkins
2024-08-06cid#555593 COPY_INSTEAD_OF_MOVECaolán McNamara
and cid#1555849 COPY_INSTEAD_OF_MOVE cid#1555936 COPY_INSTEAD_OF_MOVE cid#1555951 COPY_INSTEAD_OF_MOVE cid#1555955 COPY_INSTEAD_OF_MOVE cid#1555960 COPY_INSTEAD_OF_MOVE cid#1555964 COPY_INSTEAD_OF_MOVE cid#1555965 COPY_INSTEAD_OF_MOVE cid#1555975 COPY_INSTEAD_OF_MOVE cid#1555979 COPY_INSTEAD_OF_MOVE cid#1555987 COPY_INSTEAD_OF_MOVE cid#1555990 COPY_INSTEAD_OF_MOVE cid#1555991 COPY_INSTEAD_OF_MOVE cid#1556002 COPY_INSTEAD_OF_MOVE cid#1556008 COPY_INSTEAD_OF_MOVE cid#1556011 COPY_INSTEAD_OF_MOVE cid#1556015 COPY_INSTEAD_OF_MOVE cid#1556017 COPY_INSTEAD_OF_MOVE cid#1556023 COPY_INSTEAD_OF_MOVE Change-Id: I8ab99d8e52a1780173a4272c59d408432c29de9f Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171572 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
2024-08-04cid#1556583 COPY_INSTEAD_OF_MOVECaolán McNamara
and cid#1556585 COPY_INSTEAD_OF_MOVE cid#1556588 COPY_INSTEAD_OF_MOVE cid#1556593 COPY_INSTEAD_OF_MOVE cid#1556597 COPY_INSTEAD_OF_MOVE cid#1556605 COPY_INSTEAD_OF_MOVE cid#1556617 COPY_INSTEAD_OF_MOVE cid#1556635 COPY_INSTEAD_OF_MOVE cid#1556790 COPY_INSTEAD_OF_MOVE cid#1556792 COPY_INSTEAD_OF_MOVE cid#1556796 COPY_INSTEAD_OF_MOVE cid#1556799 COPY_INSTEAD_OF_MOVE cid#1556815 COPY_INSTEAD_OF_MOVE cid#1556836 COPY_INSTEAD_OF_MOVE cid#1556840 COPY_INSTEAD_OF_MOVE cid#1556842 COPY_INSTEAD_OF_MOVE cid#1556859 COPY_INSTEAD_OF_MOVE cid#1556860 COPY_INSTEAD_OF_MOVE cid#1556866 COPY_INSTEAD_OF_MOVE cid#1556869 COPY_INSTEAD_OF_MOVE cid#1556870 COPY_INSTEAD_OF_MOVE Change-Id: I3df8698a4aecbb03999c084517e37e12ff46ee97 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171435 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
2024-07-30try to fix crash in JunitTest_svx_unoapiNoel Grandin
Seen by vmiklos locally, and by myself on jenkins. I cannot reproduce this locally, so this is somewhat of a blind fix. Program terminated with signal SIGSEGV, Segmentation fault. $#0 0x00007f3e995c273a in sdr::properties::AttributeProperties::Notify (this=0x7f3e7000deb0, rBC=..., rHint=...) at /home/vmiklos/git/libreoffice/core/svx/source/sdr/properties/attributeproperties.cxx:464 464 pNewStSh = static_cast<SfxStyleSheet*>(rModel.GetStyleSheetPool()->Find( [Current thread is 1 (Thread 0x7f3e6d2566c0 (LWP 18774))] (gdb) bt $#0 0x00007f3e995c273a in sdr::properties::AttributeProperties::Notify(SfxBroadcaster&, SfxHint const&) (this=0x7f3e7000deb0, rBC=..., rHint=...) at /home/vmiklos/git/libreoffice/core/svx/source/sdr/properties/attributeproperties.cxx:464 $#1 0x00007f3e995bd674 in sdr::properties::TextProperties::Notify(SfxBroadcaster&, SfxHint const&) (this=0x7f3e7000deb0, rBC=..., rHint=...) at /home/vmiklos/git/libreoffice/core/svx/source/sdr/properties/textproperties.cxx:549 $#2 0x00007f3e9b421176 in SfxBroadcaster::Broadcast(SfxHint const&) (this=0x7f3e683ce5f0, rHint=...) at /home/vmiklos/git/libreoffice/core/svl/source/notify/SfxBroadcaster.cxx:40 $#3 0x00007f3e9b35fdad in (anonymous namespace)::StyleSheetDisposerFunctor::Dispose(rtl::Reference<SfxStyleSheetBase>) (this=0x7f3e6d2532a0, styleSheet=rtl::Reference to 0x7f3e6830c260) at /home/vmiklos/git/libreoffice/core/svl/source/items/style.cxx:762 $#4 0x00007f3e9b31b2bd in svl::IndexedStyleSheets::Clear(svl::StyleSheetDisposer&) (this=0x7f3e685ebd50, disposer=...) at /home/vmiklos/git/libreoffice/core/svl/source/items/IndexedStyleSheets.cxx:187 $#5 0x00007f3e9b35fe8d in SfxStyleSheetBasePool::Clear() (this=0x7f3e683ce5f0) at /home/vmiklos/git/libreoffice/core/svl/source/items/style.cxx:773 $#6 0x00007f3e9b35f0d7 in SfxStyleSheetBasePool::~SfxStyleSheetBasePool() (this=0x7f3e683ce5f0, __in_chrg=<optimized out>) at /home/vmiklos/git/libreoffice/core/svl/source/items/style.cxx:580 $#7 0x00007f3e4c4d217e in __static_initialization_and_destruction_0(int, int) (__initialize_p=32574, __priority=1748821488) at /usr/include/c++/12/iostream:74 $#8 0x00007f3e4c4cedba in std::construct_at<ScSortKeyState, ScSortKeyState>(ScSortKeyState*, ScSortKeyState&&) (__location=0x7f3e683ce5f0) at /usr/include/c++/12/bits/stl_construct.h:94 $#9 0x00007f3e683ce5f0 in () $#10 0x00007f3e6d253380 in () $#11 0x00007f3e4c4ceddc in std::construct_at<ScSortKeyState, ScSortKeyState>(ScSortKeyState*, ScSortKeyState&&) (__location=0x7f3e4c4ceddc <std::construct_at<ScSortKeyState, ScSortKeyState>(ScSortKeyState*, ScSortKeyState&&)+36>) at /usr/include/c++/12/bits/stl_construct.h:97 $#12 0x00007f3e9e756ce4 in cppu::OWeakObject::release() (this=0x7f3e683ce668) at /home/vmiklos/git/libreoffice/core/cppuhelper/source/weak.cxx:229 $#13 0x00007f3e4c460e26 in std::__uniq_ptr_impl<SvNumberFormatter, std::default_delete<SvNumberFormatter> >::operator=(std::__uniq_ptr_impl<SvNumberFormatter, std::default_delete<SvNumberFormatter> >&&) (this=0x7f3e683ce668, __u=...) at /usr/include/c++/12/bits/unique_ptr.h:184 $#14 0x00007f3e4c460c30 in rtl::Reference<ScStyleSheetPool>::~Reference() (this=0x7f3e683ce5f0, __in_chrg=<optimized out>) at /home/vmiklos/git/libreoffice/core/include/rtl/ref.hxx:126 $#15 0x00007f3e4c460175 in ScPoolHelper::ScPoolHelper(ScDocument&) (this=0x7f3e683ce5f0, rSourceDoc=...) at /home/vmiklos/git/libreoffice/core/sc/source/core/data/poolhelp.cxx:33 $#16 0x00007f3e4c4601fc in ScPoolHelper::ScPoolHelper(ScDocument&) (this=0x7f3e682a3370, rSourceDoc=...) at /home/vmiklos/git/libreoffice/core/sc/source/core/data/poolhelp.cxx:34 $#17 0x00007f3e4bddcf52 in cppu::OWeakObject::operator delete(void*) (pMem=0x7f3e4bddcf52 <cppu::OWeakObject::operator delete(void*)+3>) at /home/vmiklos/git/libreoffice/core/include/cppuhelper/weak.hxx:90 $#18 0x00007f3e682a3370 in () $#19 0x00007f3e6d253480 in () $#20 0x00007f3e4c0bda00 in VclPtr<VirtualDevice>::disposeAndClear() (this=Python Exception <class 'gdb.MemoryError'>: Cannot access memory at address 0xffffffffffffffd8 Change-Id: I936b5042a368b08a20ea9f545892ef8a6625f5fc Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171235 Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk> Tested-by: Noel Grandin <noel.grandin@collabora.co.uk>
2024-07-29tdf#161846 use unordered_map in SfxItemPropertyMapNoel Grandin
with large property maps, even a binary search starts showing up, but we can do a O(1) search here by using a map Change-Id: Ie7916076073e6dd393f0a1fb5a0db1b973999408 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171173 Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk> Tested-by: Jenkins
2024-07-29use more GetItemSurrogatesForItemNoel Grandin
since it is considerably more efficient Change-Id: I224ff890f6dd52481621b46f912f1e8dbf65126c Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171182 Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk> Tested-by: Jenkins
2024-07-29Use NDEBUG to show/hide m_bDeletedOliver Specht
"#ifndef NDEBUG" is now always used to hide/show member m_bDeleted Change-Id: I10f5111cd36e13b8101d2a69ed9268bee622344a Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171012 Tested-by: Jenkins Reviewed-by: Oliver Specht <oliver.specht@cib.de>
2024-07-29tdf#161875 buffer NameOrIndex Items for fast SurrogatesArmin Le Grand (Collabora)
Problem is that collecting the Items using the ItemSets and PoolItemHolders is too expensive when used too often. For read-only access it is okay to have the Items directly registerd (for write access we *need* the ItemSets and PoolItemHolders, see iterateItemSurrogates). This is limited to Items which need to support surrogates, but can further be limited to the here critical ones - the ones derived from NameOrIndex. This is done here by checking if the Item is a NameOrIndex based one by adding a bool to the item that gets set in the NameOrIndex constructor. If needed this can be changed, e.g. by using besides the SFX_ITEMINFOFLAG_SUPPORT_SURROGATE another flag signaling this. Since only Items that are currently held by an ItemSet or a PoolItemHolder get registered it is not necessary to change the Item's RefCount in any way, doing that may lead (again, we had that with directly set Items at the Pool in the past) to long-living Items that only get cleaned-up when the pool/ document gets destructed. This buffering is now SfxItemType-based, no longer using the WhichID, so the result needs to be checked for WhichID additionally - if needed (?). All in all it's anyways a compromize, every usage of the surrogate mechanism is a 'hack' in the sense that for lazy reasons not the model data is traversed directly, but assumed that all Items set at a pool/model *are* model/document data (what is not always true). CheckNamedItem does not need to be static, changed that. Also all accesses to maRegisteredNameOrIndex *have* to work on the MasterPool instance, same as buffered ItemSets or PoolItemHolders, corrected that, too. Number of instances in the buffer need to be counted, else an instance will be removed too early: The same instance of an Item can be referenced by multiple sets/holders, so the first remove from the buffer would already remove even when the Item is referenced multiple times. Added that. Added more asserts & made sure that all constructors/ destructors of SfxItemSet do take care of registering Items for the surrogate mechanism as needed. Change-Id: Ib33e7f0bd4abd32a3bb68278f33b0abb9a4754c3 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171084 Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk> Tested-by: Jenkins
2024-07-26cid#1608367 silence Overflowed return valueCaolán McNamara
Change-Id: I842a64e70814bfed4c04d25734a8b43dca85ca15 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171059 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
2024-07-22Improve the perf for pool item scanning..Noel Grandin
for large complex documents with lots of shapes. When that happens, we spend a lot of time scanning the std::unordered_set inside DefaultItemInstanceManager. Since most of our items are already capable of being hashed, and thus avoiding the scanning cost, make it so we can use the HashableItemInstanceManager most of the time. Change-Id: I43f4c04e956d316c976bea67d1941529d2d91182 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170813 Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk> Tested-by: Jenkins Tested-by: Armin Le Grand <Armin.Le.Grand@me.com> Tested-by: Noel Grandin <noel.grandin@collabora.co.uk>
2024-07-21Pull today DateTime out of loopEike Rathke
It's unnecessary to obtain system time twice or thrice and also prevents different today values when looping over midnight. Change-Id: Ifcf4917769bb1d853e326049711e295d54408eef Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170699 Tested-by: Jenkins Reviewed-by: Eike Rathke <erack@redhat.com>
2024-07-20tdf#161729 clear style sheets in same order as they were addedPatrick Luby
std::vector::clear() appears to delete elements in the reverse order added. In the case of tdf#161729, a style sheet's SfxItemSet can have a parent SfxItemSet and that parent is the SfxItemSet for a style sheet added later. Deleting from the end of the vector deletes a style sheet and its SfxItemSet. If the now deleted SfxItemSet is a parent SfxItemSet of a style sheet that was added earlier, the style sheet added earlier will now have an SfxItemSet with its parent set to an already deleted pointer. And so a crash will occur when that earlier style sheet is deleted. rxStyleSheet.clear(); Change-Id: I8ce7023fce8b01432cb3c9288a8f83e7a2f0f2d8 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170707 Tested-by: Jenkins Reviewed-by: Patrick Luby <guibomacdev@gmail.com> Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2024-07-19cid#1608321 Double lockCaolán McNamara
Change-Id: I831db1632e2a8fab9194ffb54df61f55e6304864 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170748 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
2024-07-18make date/time single-arg constructors explicitNoel Grandin
Change-Id: I943f755d95b90ef8aeb7d5b339f617ff50db4e29 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170673 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2024-07-14cid#1555646 COPY_INSTEAD_OF_MOVECaolán McNamara
Change-Id: I477cb9b74c99b32eb06e054fa38dd3ffa6bb77da Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170447 Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com> Tested-by: Jenkins
2024-07-14cid#1608181 Double lockCaolán McNamara
Change-Id: Id6121213f11c4ce0c855a0937fd2d7c81946c076 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170446 Tested-by: Caolán McNamara <caolan.mcnamara@collabora.com> Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
2024-07-11ITEM: replaced typeid/hash_code with SfxItemTypeArmin Le Grand (allotropia)
for the ItemInstanceManager the identification was done using typeid/hash_code, but that has problems - it is only defined to be identical for two instances of the same type, but other types can have the same code. Thanks to Noel to find that out, that is not reliable to be used. In the meantime we have SfxItemType to identify Item instances, so I changed the code to use that. The master had five additionally added Items that use the (2a) method of this mechanism (see comments in svl/source/items/globalpool.cxx for that). The gloal is to use as less as possible of this Items in that mechanism, so I checked. For four of these hashing was used, so the mem/runtime aspect is okay -> access is fast enough to prefer mem over runtime. Adding hashed Items (or any other fast mechanism) is always okay. One did not have that (SvxBoxItem) and used just the fallback ItemInstanceManager, so I removed it. We now have 18 Items for which that mechanism is initialized immediately. Noel also already moved the countdown for starting to use the mechanism in case of default handling (case (1), NUMBER_OF_UNSHARED_INSTANCES) to the unorderd_set. someting I had already planned to do, too - thanks! Change-Id: Icf6f427e5ea64672f385357aaad75bb5b7fcbf98 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170314 Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk> Tested-by: Jenkins Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
2024-07-09Revert "fix and simplify the ItemInstanceManager mechanism"Noel Grandin
This reverts commit 85fd526fc681a994415bb422090d1d23aa7d54f6. Change-Id: I5019f72f88497f50a77666d57f2d16c2749bd1c9 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170218 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2024-07-08make some of the SfxStringItem subclasses hashableNoel Grandin
Change-Id: I7ad748b67266926f1e4e67e843a90b5ac3fe58b5 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170152 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2024-07-06cid#1609596 silence Unintended sign extensionCaolán McNamara
Change-Id: Ia0e95e0dd82e28f155ddf79c48832a9e0ddc3d98 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170024 Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com> Tested-by: Jenkins
2024-07-06cid#1609597 silence Using invalid iteratorCaolán McNamara
Change-Id: I08e9ee6016ca0d67be76b0a04e7205e6d90bb46e Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170023 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
2024-07-06use o3tl::sorted_vector in DefaultItemInstanceManagerNoel Grandin
Change-Id: I16426008dd8983d56a49b3334b3f163ec350be0e Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170057 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2024-07-05fix asan buildNoel Grandin
after commit 85fd526fc681a994415bb422090d1d23aa7d54f6 "fix and simplify the ItemInstanceManager mechanism" The problem is that some *Item classes in sw/ and sc/ share WhichIds, and a whole bunch of SfxBoolItem subclasses share the same SfxItemType enum value. So we ended up mixing and matching objects of different concrete subclasses in a given *ItemManager collection. Add some asserts to the global pool code to catch issues like this earlier on. Add unique value of the SfxItemType enum for all the SfxBoolItem subclasses Change-Id: I3c8d4e02be1cd412b0292e973a6498df5f8e7102 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170003 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2024-07-03Fix typoAndrea Gelmini
Change-Id: Ib5752f1301c95d3941ecacac13918184dfe7c945 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169935 Reviewed-by: Julien Nabet <serval2412@yahoo.fr> Tested-by: Julien Nabet <serval2412@yahoo.fr>
2024-07-03fix and simplify the ItemInstanceManager mechanismNoel Grandin
The mechanism is currently broken because it uses hash values as keys, in two different places. But hash values are not required to be unique, and as soon as there are enough items of a given type, a collision is inevitable. So just simplify this whole mechanism. There is no reason we need type-specific item managers. Stuff everything into a single global pool, that uses hashing correctly. Notes (*) Performance, as far as I can tell, is the same or slightly better. (*) I removed the NUMBER_OF_UNSHARED_INSTANCES thing, in favour of just having a simpler mechanism Change-Id: I9068baf9bf6fae2500ae5748c6d1aabe6c3a18a4 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169709 Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk> Tested-by: Jenkins
2024-07-01move global item pool to own source fileNoel Grandin
Change-Id: I91365c844370ef423630d5679cadd91cbf0597b0 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169799 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2024-06-26speed up DefaultItemInstanceManagerNoel Grandin
we can store the registered items in a map indexed by which-id, and avoid most of the search cost Change-Id: Ib3fbed436bc034e603819cfef8223dcc77eb7f06 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169528 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2024-06-20tdf#144208 speedup doc with lots of redline(13)Noel Grandin
Add a custom method to SfxItemSet, to avoid some of the function calling overhead in a hot loop Change-Id: I525c9a696af941c6e39aa1677eb2a85f69c621bf Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169271 Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk> Tested-by: Jenkins
2024-06-19Fix typoAndrea Gelmini
Change-Id: If21c8e5bf48da78b505f826513673c5da0df2538 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169165 Tested-by: Julien Nabet <serval2412@yahoo.fr> Reviewed-by: Julien Nabet <serval2412@yahoo.fr>
2024-06-18tdf#131562: move test to svl and extend itXisco Fauli
based on the commit message from 7186541219599e1b51ad35601c2cd015a329f360 "Resolves: tdf#131562 decimal separator may not be surrounded by blanks" Change-Id: I19c2a687663304003566e9d93504f0baf33f1d83 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169111 Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org> Tested-by: Jenkins
2024-06-18tdf#48706: svl_qa_cppunit: Add unittestXisco Fauli
Change-Id: Ia57006f88c8d0d0658178edb4d645b9d63de37c7 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169089 Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org> Tested-by: Jenkins
2024-06-18Add SfxItemType to SfxPoolItemOliver Specht
The SfxPoolItem has a new member SfxItemType m_eItemType to compare types based on enums instead of typeinfo() which consumes a lot of time e.g. while AutoFormat is running Change-Id: I033ce67bc9a28ee4790f162380314de85fb4154e Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166452 Tested-by: Jenkins Reviewed-by: Thorsten Behrens <thorsten.behrens@allotropia.de> Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
2024-06-17cid#1603617 Big parameter passed by valueCaolán McNamara
and ~20 of similar warning The current size of SfxItemSet is 144 bytes, and std::function is 32 bytes of that. If we reintroduce Changed as a virtual method we can avoid the need for this callback. All of the calculation work that was originally unconditionally done, and then thrown away, was moved into the specific SwAttrSet case of this so the other normal cases don't do any wasted work anymore. Change-Id: Ieec90f6d28dad8a6bf1cf8f402042812bd81c331 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/168967 Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com> Tested-by: Caolán McNamara <caolan.mcnamara@collabora.com>
2024-06-13tdf#159930 Treat special case with no integer digitLaurent Balland
Number format with no integer digit, such as "+.##;-.##" was placing string with the decimal separator. This change insert string at the begining of the number string as expected. This format code is saved as "+#.##;-#.##" in ODF, but display is the same; so no change is made on this side. Add unit test for this format Change-Id: I74fbe0e9a5303672ac7927d37922c06a762feba6 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/168577 Reviewed-by: Eike Rathke <erack@redhat.com> Tested-by: Jenkins
2024-06-12Fix typoAndrea Gelmini
Change-Id: I4dd1f818a0b9e14ec0ec48533b0dc3682ff9e407 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/168735 Reviewed-by: Julien Nabet <serval2412@yahoo.fr> Tested-by: Julien Nabet <serval2412@yahoo.fr>
2024-06-11ITEM: Change SfxItemSet to use unordered_setArmin Le Grand (allotropia)
With all the changes done for Items we can now do deeper basic changes to the ItemSet itself with manageable risk. I already did https://gerrit.libreoffice.org/c/core/+/166455 aka 'ITEM: Add measurements for SfxItemSet usages' to get some statistical information about the fill/usage grade of the ItemSet's fixed PtrArray to SfxPoolItems, check that out to get an own picture. Those results show that an average usage is between some extremes ranging from 0% to 50%, but when using more checks and using multiple files/interactions/edits in all applications we end up with around typical 12%-19% of that array used, the rest is nullptr's. Thus I thought about one of the initial ideas of this series of changes (ITEM), to use a std::unordered_map (A) instead of that fixed array of SfxPoolItem Ptr's (B). Tthat again was for a complete type-based rewrite, which I once did a POC, but the code cannot be adapted to that, just too much work. Those are very different in architecture, (B) is done since a long time (since ever), but as pointed out above, (A) is now possible. There are many aspects to it, let's grep some: Speed (iterate): (A) and (B) are both linear. (A) has less entries, but may touch different mem areas (buckets). (B) is linear, but many empty spaces which are usually uselessly iterated. Speed (access Item by WhichID): (A) is hashed by WhichID, so mostly linear for unordered_set/hash. (B) is in a linear array, but has to calculate the offset for each WhichID access. So I guess speed will mostly equal out. Memory: (A) will be dynamically allocated (buckets), but stl is highly optimized and may even re-use areas, has to provide some extra info but will need less places for Items since it's dynamic and can start empty. (B) will be allocated once (except AllItemSet) and may even be 'derived' to the ItemSet (SfxItemSetFixed), but has to allocate all space at once. I can go in lots of more detail here, but due to the results of the statistics I just made a test now, including measuring some results (will include in gerrit, not here). I used two pro versions for that. That way I have some data now. Result is: - It is now possible to do this, it runs stable :-) - Speed: As expected, mostly no change - Memory: Depending on usage, 0% to 25% less, usually around 8-10% Side effects: - SfxAllItemSet could be done without WhichRanges, thus without expensive 'merges' - SfxItemSetFixed is not needed. While the idea is good, it needs a lot of extra stuff - Access to Items is linear if set - WhichRanges: Still needed, but for vaildity checking/filtering of ItemSet content - WhichRanges: Worth to think about if these are needed at all, probably just exist for historical reasons since allocation/number of added Items was never ever dynamic -> just not allocatable Putting the current version on gerrit, may still trigger some UTs (checked SW/SC/SD...) I did not like that functionality at ItemSet that hands out a vector of the set items for cases where to avoid iterating and deleting items at the same time at an ItemSet, so changed to using a local vector of remembered WhichIDs and deleting after the iterator is done. I also saw some strange usages of SfxItemIter in sw which i will have to check. Since there are still problems with UTs which I can not reproduce locally I have now added asserts to the case that an ItemSet gets changed with still having active SfxItemIter(s). That is always an error, but with the architecture of that fixed array of ItemPtrs did not have devastating effects (purely by coincidence). With that asserts, UTs run well in SC and SD, but in SW I get 11 (eleven!) asserts from the UTs, all of them from 'ITEM: SfxItemSet ClearItem' BTW. I guess these have to be fixed before thinking about this change... Good news is that all 11 cases were the same in SW, in SwHistorySetAttrSet::SwHistorySetAttrSet which does some strange things using two SfxItemIter in parallel. Thus SW UTs are also clear and I see no more errors caused by ItemSets being changed while SfxItemIters are alive. Bad news is that I still have errors to hunt... NOTE: Have now cleaned all UTs, this showed that there are some unexpected side-effects of the Items being processed in another order when SfxItemIter is used, also found one case where a WhichRangesContainer is constructed for a SfxItemSet using the set items from another ItemSet and SfxItemIter to do so. There *might* be more cases not covered by UTs. NOTE: While speed stays the same and mem is reduced up to 25% (see measurements in 1st comment) another *important* aspect is that this frees the way for using ItemSets *without* WhichRanges - these are necessary mainly to create that fixed array of pointers to the Items in a *manageable* size. With a dynamic structure like unordered_set there is in principle *no need* anymore to use WhichRanges to pre-define the Items a Set could hold. There is one exception: We have cases where one ItemSet is set at another one with defined WhichRanges to *filter* the contained Items - these would have to be identified. This is a rare case and we would have to check cases where an ItemSet gets set at another ItemSet. This would be as if all ItemSets would be AllItemSets in principle - much easier for everyone. NOTE: Waited for 24.8 split just to not take unnecessary risks. Change-Id: I75b0ac1d8a4495a3ee93c1117bdf618702785990 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166972 Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com> Tested-by: Jenkins
2024-06-06Resolves: tdf#161430 reindex the correct style if there are duplicate namesCaolán McNamara
Change-Id: I6d4e96faef3ec6caa038edf7595f91f20d964807 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/168479 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
generated by cgit (git 2.34.1) at 2024-12-24 19:31:52 +0000