Add option to disable release_target_info

[acmorrow]

Describe the Feature The release_target_info method attempts to reduce the peak memory footprint of the build, but at the cost of re-invoking the changed method. This isn't free. An option to disable this behavior could make for faster builds when developers are working on machines with plenty of memory.

Required information Discussed directly with Bill.

• Version of SCons 3.1.1

• Version of Python Python 3.6

• Which python distribution if applicable (python.org, cygwin, anaconda, macports, brew,etc) N/A

• How you installed SCons Vendored into project

• What Platform are you on? (Linux/Windows and which version) Linux Ubuntu 18.04

• How to reproduce your issue? Please include a small self contained reproducer. Likely a SConstruct should do for most issues. Do a no-op build of mongodb and time it. Then make release_target_info into a no-op and retime. You should see that about 30 seconds of wall clock time is saved.

• How you invoke scons (The command line you're using "scons --flags some_arguments")

python3 src/third_party/scons-3.1.1/scons.py --implicit-cache --build-fast-and-loose=on --dbg=on --opt=on --variables-files="/home/andrew/.scons/site_scons/mongo_custom_variables.py" --link-model=static --install-mode=hygienic -j24 CC=/usr/bin/clang-7 CXX=/usr/bin/clang++-7 install-all-meta CCFLAGS=-gsplit-dwarf --cache --debug=time

Additional context Ideally, this would be controlled by a command line switch, and allow SetOption to configure the value so it can be programmatically changed from within a running SConstruct.

When running with doing the release: 142.63user 2.06system 2:24.66elapsed 100%CPU (0avgtext+0avgdata 824140maxresident)k Virtual memory: ~12 GB Peak RSS: ~805 MB

When running without doing the release: 128.64user 2.53system 2:11.16elapsed 100%CPU (0avgtext+0avgdata 843256maxresident)k Virtual memory: ~12 GB Peak RSS: 824 MB

It seems to save 20MB of memory but cost 15 seconds (I've seen as much as 30). That doesn't seem like a good trade-off on modern systems. I'm not sure it even does anything in our build.

[acmorrow]

Having thought about this a little more, if the memory savings for a MongoDB build (which is a fairly large project I think?) are all of 25 MB, I think I'd propose simply removing the release_target_info subsystem entirely. Memory is cheap.

[bdbaddog]

we can't do that without a deprecation cycle. Best way to do that would be:

1. Add a flag to control it
2. Change default to not free
3. Assuming no one has an issue, remove the option and remove the logic.

[acmorrow]

Well, release_target_info isn't really user facing is it? So I don't see why it couldn't just be removed. If there is worry that maybe some people have accessed the symbol despite it being internal (to monkey patch it or similar) then If it just became a no-op for a while that would still work too. I guess the keep_target_info attribute could be considered user facing, but in that case you could just deprecate that attribute and still just make the default be to not release memory. It seems like an implementation detail.

[bdbaddog]

It's possible we've still got users stuck in 32bit world... where it could matter. When this was implemented for some sample build I think it was saving 100mb's of peak size.. which kept it from swapping.

I'd rather (minimall) add the option and default off.. before removing the logic.. less painful to unravel

[acmorrow]

True. I sort of forgot that 32-bit exists but of course it does. You could add it as an option but default it differently depending on 32-bit vs 64-bit python.

[bdbaddog]

We'd have to default to current behavior, and then deprecate to off by default or off on 64 bit only.

[acmorrow]

@bdbaddog - So what would the right sequence of activities look like for this ticket in order to achieve a deprecation cycle? Ideally it would look like something where initially (4.0?) there was a flag --release-target-info that defaulted to True. And then eventually, later, defaulted to False? Where and when does the deprecation happen?

[bdbaddog]

We've got a wiki page on that :) https://github.com/SCons/scons/wiki/DeprecationCycle

But yes as you said, add feature where default is current behavior. announce it's going to change in next release (4.1.0 probably, might 5.0.0 but I don't think so), then in that release flip default. I'd guess at this point that few would note and complain about flipping the default, but I've been surprised before.

[mwichmann]

Hmmm, this feature is actually fairly recent... RELEASE 2.3.1. Although I guess that's a decade ago, so not so new.

Should be easy to put this in, the code already checks for self.attributes.keep_targetinfo so presumably (?) it's just a question of being able to set that.

[bdbaddog]

Hmmm, this feature is actually fairly recent... RELEASE 2.3.1. Although I guess that's a decade ago, so not so new.

Should be easy to put this in, the code already checks for self.attributes.keep_targetinfo so presumably (?) it's just a question of being able to set that.

I think that's a per Node setting, the idea would be to totally skip releasing the info. The logic was added a while back to reduce the maximum RAM footprint as machines were running out of memory (pre 64 bit, > 4G RAM common in a build machine days)

[mwichmann]

Darn, you're right (that it's per-node)

[acmorrow]

@bdbaddog - I think it is somewhat regrettable that we need to wait a whole deprecation cycle to make things faster on the common platform (64-bit, lots of mem) to preserve memory savings for big builds on 32-bit. What if, as a compromise, we added a --release-target-info flag now, and had it default to off for 64-bit environments but still default to on for 32-bit environments, but also deprecate that 32-bit default. Then, after the next release, flip it to off everywhere. That way the next release would be faster for 64-bit builds, 32-bit builds would see no change (except a deprecation warning on startup?), and then in the next next release builds that wanted the memory saving would need to opt into ---target-info?

[acmorrow]

Also, just bikeshedding here, but the flag could also be something like --reduce-memory-usage or --optimize-for=[performance|memory] since there may be other places in the future where a performance vs. memory trade-off might make sense.

[bdbaddog]

I"m not sure we'd need a deprecation cycle for this? It's a undocumented internal function. That said deleting it wholesale could impact users where memory is tight, not sure how many/if any still are in this situation.