kicad-developers team mailing list archive
Mailing list archive
Re: Boost include files.
On 9/15/2010 8:45 PM, Dick Hollenbeck wrote:
>>>>> I probably would never personally use shared_ptr because in my mind it
>>>>> is slightly beyond what an average C++ programmer uses on a day to day
>>>>> basis, and it obscures the clear notion of "object ownership".
>>>>> I have never (I am old, this is a long long time, and countless lines of
>>>>> code) been in a position where I could not assign object ownership
>>>>> clearly to one container over another. If ever this became obscure, I
>>>>> would probably backup and take another look.
>>>> I'ld rather not get off topic but I think I may have found one as I have
>>>> wrestled with a problem for several weeks and with at least two false starts.
>>>> The problem stems from the current design of CMP_LIBRARY and it's ownership of
>>>> LIB_ALIAS and LIB_COMPONENT objects and the interdependency between them. My
>>>> original attempt was to make CMP_LIBRARY own LIB_COMPONENT objects and
>>>> LIB_COMPONENT own it's associated LIB_ALIAS objects. This made both searching,
>>>> indexing, and modifying the library and editing components much more complex
>>>> than the current design so I abandoned it as one of my goals was to simplify
>>>> the current design. I finally decided that CMP_LIBRARY should only own
>>>> LIB_ALIAS objects. I created an addition LIB_ALIAS object for the root
>>>> LIB_COMPONENT(more on this below). I added a boost::shared_ptr for the
>>>> LIB_COMPONENT pointer in each LIB_ALIAS that references it and I added code in
>>>> the LIB_ALIAS destructor to keep the LIB_COMPONENT name up to date. When the
>>>> last LIB_ALIAS that references the shared LIB_COMPONENT is deleted, the
>>>> LIB_COMPONENT object goes out of scope and is automatically deleted. This
>>>> makes sense to me as LIB_ALIAS objects no meaning without an associated
>>>> LIB_COMPONENT. It also made the code for updating the CMP_LIBRARY object very
>>>> simple. Access to the underlying LIB_COMPONENT object is always done though
>>>> LIB_ALIAS::GetComponent(). I know your thinking why create an alias with the
>>>> same name as the component? This change will allow for future expansion, if we
>>>> want to support custom fields in aliases for the so called heavy weight
>>>> libraries (if I am correctly understanding what the library development folks
>>>> are requesting). This will probably make more sense when I submit the
>>>> blueprint for the new component library file format which should be in the next
>>>> week or so. Except for the additional boost header files, the changes to the
>>>> current BZR version are pretty minimal in keeping with my preference to making
>>>> small incremental changes to the existing code. I could submit a patch or
>>>> publish my branch if you would like to take a look at it. If you have any
>>>> suggestions that can improve this design, I would be more than happy take a
>>>> look at it.
>>> The relationship between objects as you describe it is elegant. No
>>> objections there.
>>> I personally would not used shared_ptr myself. I'm not saying don't use
>>> it because I am respectful of your time and judgment.
>>> Just looking the source code and the depth of nesting of the headers
>>> used to pull off shared_ptr.hpp makes my head spin.
>>> You asked for an alternative, will this work? :
>>> In lieu of shared_ptr my approach would entail a factory method and
>>> destruction method that would be responsible for creating LIB_COMPONENT
>>> in the context of CMP_LIBRARY, even if this entailed overloading new and
>>> delete on LIB_COMPONENT. Anytime a LIB_COMPONENT is created, add its
>>> LIB_COMPONENT* to a registry housed in the CMP_LIBRARY. Anytime one is
>>> deleted, remove the LIB_COMPONENT* from that registry.
>> My first attempt was to let the CMP_LIBRARY own the LIB_COMPONENT
>> objects but it made the sorting, searching, and fetching names code more
>> complex because you have to get the aliases for each component. This is
>> important for listing names in the library viewer and library editor
>> code. So I abandoned that that idea as trading one set of complexity
>> for another.
>>> Candidates for the registry container are:
>>> A) std::set<LIB_COMPONENT*> or
>>> B) std::map<LIB_COMPONENT*, int>
>> I like the idea using of a map as a temporary holder for LIB_ALIAS
>> object pointers associated with a LIB_COMPONENT. Using std::map<
>> wxString, LIB_ALIAS*> would allow you to use destructor of LIB_ALIAS to
>> decouple it from it's associated LIB_COMPONENT before it gets deleted.
>> This could also provide a handy method for simplifying the library
>> editor code.
> No, my suggestion is no different than what you originally described.
> I would not continue, but it seems you misunderstood what I was saying.
> Feel free to disagree with me, but in order to do that one must first
> understand what was said.
> Let's not confuse ownership with having a pointer to something.
> Ownership in my mind is the "obligation to delete". It is not an
> exclusive license on holding a pointer to something, any number of
> LIB_ALIASes can point to a LIB_COMPONENT, which is what you described,
> and is what I am describing. If I am a CMP_LIBRARY, and I own any number
> of LIB_COMPONENTS, there is nothing keeping any number of LIB_ALIASes
> from pointing to them. Yes I also own the LIB_ALIASes as a CMP_LIBRARY.
> You still have the pointers from several LIB_ALIASes to a single
> LIB_COMPONENT, its just that there is no ownership in those pointers.
> The LIB_COMPONENTs are owned by the largest container, namely the
> class CMP_LIBRARY
> std::map<LIB_COMPONENT*, int> registry;
> registry owns the "key" part of the map pair,
> the "int" is the reference count. CMP_LIBRARY
> destructor can delete all the keys, fulfilling
> its ownership obligation.
> lib_ptr->registry[component_ptr]--; // decrements the reference count.
> But to be honest, reference counting may not even be needed?
> There would probably be no harm in never deleting the LIB_COMPONENT
> until the the CMP_LIBRARY destructor was called. Who cares if some
> LIB_COMPONENTs stay instantiated that are no longer used by your
> aliases? They are essentially invisible when the last LIB_ALIAS stops
> pointing to them. This might make disk saving a little harder, but it
> is sort of like garbage collection in a Java Virtual Machine, save only
> the ones that still have aliases pointing to them. This could be done
> by creating a std::set<LIB_COMPONENT*> on the stack in the save
> function, and a single sweep through all the aliases and record which
> LIB_COMPONENTS still have an alias user. You can see an example of this
> in netform.cpp.
I see said the blind man. I understand now where you were going with
you previous post. I will revisit my code.
> Yes shared_ptr implements reference counting. Probably a python
> programmer wanted to make C++ into a higher level language. So what,
> maybe he should have just stuck with python? Where is that reference
> count stored? (I know the answer, but I wan't you to consider where it
> is.) How much slower will it be to compile the whole eeschema program
> because of the required header file nesting, now that we intend to
> compile shared_ptr, which itself brings in about 20 header files, into
> nearly every one of the eeschema *.cpp files? (If you take seriously my
> suggestion in the coding standard?) LIB_ALIAS will be highly visible to
> many *.cpp files, and therefore so will shared_ptr be also, because it
> is a member field.
I agree with you on the Boost header nesting. It is amazing anyone can
wrap their head around that. I tried to follow the path for the
shared_ptr and gave up after the fifth or sixth header file.
> There are some items to factor into *your* decision.
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help : https://help.launchpad.net/ListHelp