[Scons-dev] Announcement: Core changes for v2.4 ahead, Switching the Node class to using slots...

[Kenny, Jason L]

HI dirk,

I am down to three error now and it all look like something internal. Most of the issues as I see it are deal with the way SCons did things. Looking at the code, this is much cleaner and easier to read version of SCons. Very nice. A great step forward. The short of it is that I think everything looks good at the moment. I will have to do some more testing. I an interesting the memory difference I see for some of our builds that take up to 6GB of memory at the moment.

I had some thoughts to add on the nodes, but I am too busy today to write them down. 

The only quick question I have is that I noticed the environment object don't have slots. There is a memory saving here. The base environment have a lot of "static" items. Those should be in __slots__. Add a __dict__ to the env __slots__ then and there will no loss of functionality and a small memory gain. Given that in Parts I have yet to abuse the OverideEnvironment to save memory, I have to make a lot of clone env objects to make sure every "part"/component has an environment that is safe and correct for it. I would assume most large build has to do something like this, to allow value such as CPPDEFINED, etc.. to work correctly. This would be a low hanging fruit that would save some memory as well.

Do you have any concerns over doing this?

Jason


-----Original Message-----
From: Kenny, Jason L 
Sent: Tuesday, March 31, 2015 2:07 PM
To: dl9obn at darc.de; SCons developer list
Subject: RE: [Scons-dev] Announcement: Core changes for v2.4 ahead, Switching the Node class to using slots...

>>for extending Nodes there already exists a "dict" named "attributes". It would be cool if you could >>rework your patch to use it, instead of reintroducing a dict to the Base class.

"attributes" was added for the original Tag() API to help with RPM builders. I updated this with a MetaTag in Parts that uses this to add random values. The monkey patching needs to access to the node the real Node object. 

So let me think this out load here. The main issues is that I have properties that I am adding to the node objects and this fails with __slots__ being defined without a __dict__,  because I was adding a cache value to the object, vs having to look up the value on a different object ( ie the attributes object) as this could conflict with who-knows-what that was added with the Tag()/MetaTag() API by the user.

The plus side for having __dict__ added to the base is that it makes it easy to see what is being added to a node and it makes it easy to prototype new stuff in a system. Keep in mind that point of __slots__ is not about removing __dict__, but allowing python to be efficient in pre-allocating memory as an array vs that of independent items as they would be in the __dict__ object. Ideally Cpython should be doing something else here, so __slots__ add no value. ( hmm somebody should fix this...) But until then __slots__ add value for this reason. Add __dict__ by default only mean that the code as any normal python object. One can argue that the restriction we add by using slots in add new member dynamically is a good thing or a bad thing. I just want to point out that the use of __slots__ means that the memory usage is reduced because of block array allocation ( vs lots of small alloc), and the memory layout of the object is more machine friendly ( as any CPU like array for quick access, and hate everything else). This is why we have a space and speed improvement in SCons with the nodes as the node are the prime data structure when we build.

Let me look at what I can do on my end, with a fresh set of eyes.. if you still feel that adding __dict__ is a bad thing. ( will miss the clean property syntax )

There are a few other fixes I have here in parts/overrides/nodes.py that would be nice to have added to the main base. Could you look at these and see if any seem OK for you to add to the Node classes?

Thanks,
Jason


-----Original Message-----
From: Scons-dev [mailto:scons-dev-bounces at scons.org] On Behalf Of Dirk Bächle
Sent: Tuesday, March 31, 2015 12:31 PM
To: 'SCons developer list'
Subject: Re: [Scons-dev] Announcement: Core changes for v2.4 ahead, Switching the Node class to using slots...

Hi Jason,

On 31.03.2015 16:35, Kenny, Jason L wrote:
> HI Dirk!
> So an update.
>
> I had a little time yesterday to look at this. I am down to 11 failures. They all seem to be the same issue.
>
> Quick summary:
>   I seem to have three issues with the branch as it is.
>
> 1) the base node did not have __dict__ in the slots definition. This 
> prevents my monkey patches from working for the nodes. I patched my 
> copy with to fix this. ( will submit this path to you)

for extending Nodes there already exists a "dict" named "attributes". It would be cool if you could rework your patch to use it, instead of reintroducing a dict to the Base class.

> 2) The scanner prog.py has a fix in it :-) this removes the need from the monkey patch in Parts. The issue I had was this monkey patch was fixed up in a way in Parts that broke with the fix. I have to look at why the monkey patch was done this way. Anyways I fixed Parts to note break in this case ( ie no monkey patch in this case). As a FYI this fix in the scanner allow the scanner to deal with lists of libs being returned.
> 3) What seems to be the last issue at the moment is in the node object with stored information. I have not looked in to as of yet. I hope to look at it today..
>

To me it looks like you are overwriting/patching Node.store_info as method directly. This isn't possible anymore, "store_info" is now an index (integer) pointing into a function map.

Best regards,

Dirk

_______________________________________________
Scons-dev mailing list
Scons-dev at scons.org
https://pairlist2.pair.net/mailman/listinfo/scons-dev