Refactor DeclarationId / Structure-pack most used types
ClosedPublic

Authored by kfunk on Sep 2 2015, 10:06 PM.

Details

Summary

Make use C++11 union (saves around 8 bytes per instance)

language: Structure-pack most used types

Make use of C++11 typed-enums and C++11 unions

sizeof-statistics (64 bit system):

  • DUChainBaseData: 24B -> 20B
  • AbstractTypeData: 32B -> 12B
  • DelayedTypeData: 40B -> 24B
  • DeclarationData: 76B -> 60B
  • DUContextData: 64B -> 56B
  • TopDUContextData: 96B -> 80B (not instantiated *that* often)

Diff Detail

Repository
R33 KDevPlatform
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kfunk updated this revision to Diff 693.Sep 2 2015, 10:06 PM
kfunk retitled this revision from to Refactor DeclarationId.
kfunk updated this object.
kfunk edited the test plan for this revision. (Show Details)
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptSep 2 2015, 10:06 PM
kfunk retitled this revision from Refactor DeclarationId to Refactor DeclarationId / Structure-pack most used types.Sep 2 2015, 10:12 PM
mwolff added a subscriber: mwolff.Sep 3 2015, 10:10 AM

generally +1, but still needs work.

language/duchain/declarationid.cpp
64 ↗(On Diff #693)

you'll also need to call the correct destructors here, afaik. otherwise we could leak the refcount of the indexed* types.

language/duchain/declarationid.h
95 ↗(On Diff #693)

break the long line

language/duchain/topducontext.h
180 ↗(On Diff #693)

here and elsewhere: I'd prefer using explicit uint8_t etc

language/duchain/types/abstracttype.h
118 ↗(On Diff #693)

why this change?

133 ↗(On Diff #693)

as I note below, this should be checked whether any language plugin makes use of the 64bit space. if not, +1. otherwise -1

language/duchain/types/typesystemdata.h
78

is this really safe? is it not up to language plugins to put data in here and maybe python or so using large modifiers?

This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.
kfunk marked 4 inline comments as done.Sep 8 2015, 7:36 PM

Now pushed:

commit e12636865ebc9c4f60d893bdeecb30a687ce10aa
Author: Kevin Funk <kfunk@kde.org>
Date: Thu Sep 3 00:03:57 2015 +0200

language: Structure-pack most used types

Make use of C++11 typed-enums and C++11 unions

sizeof-statistics (64 bit system):
- DUChainBaseData: 24B -> 20B
- AbstractTypeData: 32B -> 12B
- DelayedTypeData: 40B -> 24B
- DeclarationData: 76B -> 60B
- DUContextData: 64B -> 56B
- TopDUContextData: 96B -> 80B (not instantiated *that* often)

Differential Revision: https://phabricator.kde.org/D322

commit 730cb3923e22557b80a461d86a212ee41a5eeb91
Author: Kevin Funk <kfunk@kde.org>
Date: Thu Sep 3 00:02:56 2015 +0200

Refactor DeclarationId

Make use C++11 union (saves around 8 bytes per instance)

Differential Revision: https://phabricator.kde.org/D322
language/duchain/types/abstracttype.h
133 ↗(On Diff #693)

Sure, I checked. LanguageSpecificModifier is never used outside of kdevplatform afaics.

Each module calling AbstractType::setModifiers just reuses the common modifiers apparently. No need to extend to 64 bit at all, IMO (still plenty of space left now)

language/duchain/types/typesystemdata.h
78

See above; I didn't see a place where any custom modifiers were used. Checked with ack / LXR. No uses of LanguageSpecificModifier (which would be required) at all.