Sensenmann: Code Deletion at Scale
testing.googleblog.com
testing.googleblog.com
- during outage
- at end of year
- during audits
- when the one special customer that paid a fortune for an obscure feature decides to use it
It might be the case that none of this applies to your application, but it’s why “check what got hit recently” (3 months is an eye blink in the business and govt world) is dangerous.
This endeavour probably won't advance your career much, but making one's co-workers lives easier is worthwhile too!
There are industries where this mindset is business ending. It’s pretty much only high volume consumer markets where you can afford to have so little indifference to your customers.
The idea of deleting code without a _thorough_ review of exactly where, why, and by whom it is used for what, is a non-starter. I wouldn’t think of deleting anything unless it was truly dead, and even then I’d take it offline and archive it so I knew what code touched customer data.
I actually enjoy this stability. I previously worked for a company with tens of millions of end users (at the time, on-prem and cloud solutions) and a code base stretching back for (at the time) almost 15 years. I don’t miss the pressures that came with that.
Edit: typo
It really depends how urgent those people need that something at that moment (and how much they paid for it).
Otherwise this is just making dev life easier on the cost of their users.
Before deleting code, why not check who would call that code and why? But yeah, that is also work. (And not always worth the effort)
Not necessarily so, I'm expecting removal of dead code will enable faster delivery of new features, and also enable easier refactorings to make code more robust, because the dev team won't have to take unused features into consideration. The code we inherited is very complex and very brittle, so adding something often breaks something else and devs spend way too much time figuring out all the interdependencies. A major complaint is that the dev team doesn't deliver fast enough.
Well, for sure it does. Getting rid of bloat is always freeing energy - but only if it is really dead code. Otherwise you can introduce rare bugs, or break things in unexpected ways. And yes, sometimes the fastest way is to just try it out and see how things run. If it is not medical equipment, it might be fine even on a live system. It really depends on your project and users. But most users really like stability. Especially if they need that software at that moment, because they have deadlines as well.
As others say- not suggesting you don’t delete. Delete is the best refactoring! Sounds like you’re already doing the sensible extra work of chasing consumers to check endpoints really aren’t used under any circumstances.
It would be great if consumer-driven contracts were more widely adopted, so we could move more confidently when deleting stuff. It’s a constant source of annoyance in large orgs how quickly we lose track of dependencies. Keeping visibility of lineage and dependencies has a great payoff if you can build it in from the start.
A lot of these tools sample stack traces at intervals, collecting statistics of how often functions are used. If you had enough samples, then with very high confidence you could find code paths that are never used in production.
With that you could configure the APM to keep the first $N traces for all endpoints and then start sampling after that
That said, as kortilla already mentioned, in most cases 3 months is too short time to get a complete answer.
IIRC "Principles of Network and System Administration" by Mark Burgess has more about it, but intuitively you have to look for the largest cycles that exist, but there is also, again as mentioned by kortilla, events that doesn't happen on a schedule.
One thing I try to do is, when I come across code that I have to research is to write down in the documentation for the function (Javadoc or similar) why it exist and the conditions when it can be deleted.
You can also try to add some instrumentation code that notifies you when something is called. That way you can end up like me who, every new years day get a weird sms message at 14:00 or 14:01 from a system that no one can find, but at least I know that some system I cannot remember anymore still runs somewhere :-)
You can knock up some code that say solves a specific business problem right now. (meta:0)
But you need an environment that can take a new piece of code and deploy it and test it (meta:1)
how is that code running - this is shadingnfrom production monitoringninto QA and performance (meta:2)
Compare all the running code and its performance against the benefits of replacing code or going back to level 0 and just fixing a business problem (meta:3)
Then this death eater - meta 4 I think.
And to me this is why comments like "software needs to solve business problems" is naive - once you start using software you need more software to manage the software - it's going to grow till it consumes the business.
You can replace "software" with "people" and you'll end up with a sentence that's equally true.
You also have to include that every further feature is now _more_ expensive than before.
A: One. They are very efficient and don't have much sense of humor.
The main difference between German and English here is one of spelling: Germans like to write their compounds together, English speakers like to put spaces in between.
Writing compounds together like that wasn't always the norm in German spelling, either. Especially before the printing press rules were more fluid.
English would still be the same language, if you wrote supremecourt or summerolympics without a space. German would still be the same language, if you wrote Bahn Hof or Sommer Nachts Traum.
(And of course both languages also have hyphens.)
You would probably write "Sommer Nacht Traum" instead. The fugen-S of "Nachtstraum" only makes sense, when you connect "Nacht" and "Traum".
(English has no fugen-S, as far as I'm aware)
I am purely talking about changing the spelling. The Fugen-S is something you actually pronounce, so if you want to keep the language intact and spelling phonetical, you would keep the Fugen-s.
Yes, it might be a bit weird to you. But it's no weirder than other changes we make to words in German because of grammar. Like 'kleiner Hund' vs 'kleine Katze'. Adding an 's' on the previous word is no worse than adding an 'r'.
It's also interesting when you split up the words. For instance "gebrandmarkt" results in "brand gemarkt". So you extract the "brand" from "ge-...markt".
> It's also interesting when you split up the words. For instance "gebrandmarkt" results in "brand gemarkt". So you extract the "brand" from "ge-...markt".
English has a similar mechanic:
"Let's upcycle this trash!"
"I boot up my computer."
"We spruce this place up."
Source: I worked on Sensenmann.
https://sources.debian.org/src/qtwebengine-opensource-src/5.... https://codesearch.debian.net/search?q=package%3Aqtwebengine....
I wonder how many copies of the Python six module the Google monorepo contains.
But the Chrome codebase is generally the giant exception to everything due to the major open source components (Android too).
Ubuntu is not too bad.
As the article points out, you also have to look at what's actually run. This is the real advantage of Google infrastructure: the vertical integration so if a binary is run on Borg, or even on the command line, that can be tracked.
> Its goal is simple (at least, in principle): automatically identify dead code, and send code review requests ('changelists') to delete it.
It sends CLs (pull requests) and shows up as a commit. You get a chance to approve or deny the deletion
But relatively hard to find and work with.
It was quite pleasant to write, because both the guy who designed git, Linus Torvalds, is also a file system guy.
I was about to recreate a new version in Rust as an open source project. So I started with fixing up libfuse https://github.com/libfuse/libfuse/pulls?q=author%3Amatthias... and the Rust equivalent https://github.com/cloud-hypervisor/fuse-backend-rs/pulls?q=...
Your project is also interesting. I don't plan on ever adding write support. The old Python version was already using git as a library via gitpython, instead of shelling out via the command line. The new version will use Rust's gix.
Performance, even for the old Python version, was pretty decent. That probably came from using git via a library and being careful about fuse caching. I used the 'low level' API that libfuse provided, instead of the 'high level' one. The old version also already supported opening arbitrary commits, tags and branches, they were represented as different folders.
Another feature that's maybe important for performance: because everything was read-only, operations like OpenFile could be implemented as no-ops in such a way that the kernel doesn't even send us a request anymore.
(It was read-only in the sense that any changes would come from the git side. User initiated file system operations could not make any changes to the data.)
Old unused code is a huge problem for us. The coordination costs of trying to update company wide problems are made much more severe by old code.
I wish we had something like this. We’re large enough we’d need our own system anyway. We don’t have a monorepo, and we don’t use tools so many others do.
Programming languages would make more of a difference if deletes were happening at a more granular level, e.g. deleting unused functions, but this article doesn't touch on that.
interface Animal {}
class Cat implements Animal {}
class Dog implements Animal {}
Animal[] animals = new Dog[1];
animals[0] = new Cat(); // This will throw an error at runtime
C was my main/daily lang by age 18 or 19 and I got drunk on its power
And the preprocessor... dark arts indeed, manipulating the bits of the program itself!
Isn't that the case with all libraries? How does the monorepo help here?
Maybe, but searching any repo or combination of repos with billions of lines of code would be challenging.
Think Visual Studio "find all references", but working around the entire company's codebase, not just your current project.
> In the matter of reforming things, as distinct from deforming them, there is one plain and simple principle; a principle which will probably be called a paradox. There exists in such a case a certain institution or law; let us say, for the sake of simplicity, a fence or gate erected across a road. The more modern type of reformer goes gaily up to it and says, “I don’t see the use of this; let us clear it away.” To which the more intelligent type of reformer will do well to answer: “If you don’t see the use of it, I certainly won’t let you clear it away. Go away and think. Then, when you can come back and tell me that you do see the use of it, I may allow you to destroy it.
https://wiki.lesswrong.com/wiki/Chesterton%27s_Fence
While this tool certainly does the job of proposing code deletions, that's the easier part. The harder part is knowing why the code exists in the first place, which is necessary to know whether it's truly a good idea to remove it. Google, smartly, is leaving that part up to a human (for now).
Dead code is shit that has a build rule and no other build rules in the entire repo depend on it. Or private functions that have no callers in an application that isn't built with any kind of reflection capabilities.
Sometimes you still want it (e.g. python scripts that are used every once in a while for ad-hoc things and might go months between uses), but usually the right thing to do is productionize stuff like that slightly more (and also test it semi-regularly to make sure it hasn't broken).
Like conceptually I believe this could be wrong in both directions, since there's heavy caching of build artifacts, you can totally build a transitive dependency of some file without actually reading the file (and potentially do this for a relatively long period of time, though I don't think that will happen in practice), and stuff will regularly look through large swaths of files that aren't necessarily run.
Because piper[1] isn't a normal filesystem. It's often accessible through a FUSE-based api, so it appears like a filesystem to some users sometimes, but it can also be accessed over an RPC api. So the concept of "atime" isn't really a fit, because, well, you access the filesystem from a view that is based on your workspace, (think, akin to the git commit hash being in the filepath), and the "real" underlying file isn't necessarily on your machine.
So under a reasonable definition of atime, the atime of most files is never, because on any given arbitrary commit, you don't access/build everything.
> you cache an entire file instead of just reading it
You don't cache the file, you cache the file's outputs, keyed by a hash of the file (or all the files which are deps of a particular output). With bazel, you have a shared build artifact cache that can securely and reliably be shared across every user at a company with tens of thousands of engineers. If I build some target `:foo`, which corresponds to `foo.o`, generated from `foo.c`, but someone built `:foo` five minutes ago, as long as `:foo` hasn't changed (and you can check that the file hasn't changed without reading it because the fancy filesystem stores a hash of the file alongside the actual file), I won't actually read `foo.c` or go though the motions of building `:foo`, I can just pull `foo.o` directly from the object cache and use that in any dependencies, which means that I can build my output without ever invoking a compiler, which is really cheap and fast.
You could argue that upon reading the hash you should update the atime, but that's extremely expensive since now you have to do writes instead of idempotent reads a bunch of times a second.
[1]: https://cacm.acm.org/magazines/2016/7/204032-why-google-stor...
And also you still need to reverse from a cached output foo.o, to every transitive dependency of that, of which there could be thousands. Which can be done but requires invoking the build system to do. So it's not anything like just checking a timestamp on the file.
Do you really have an entire application binary that is only deployed and run once a year?
That's an enormously stupid waste of resources.
Do you really have an entire application binary that is only deployed and run once a year?
I have ones which are run when the need arises. It could be a year, it could be ten. But I know they work and I don't need to touch them, nor would I want to delete them.
So if we’re using it once a year, (we do have tools like that, maybe not strictly annually but certainly rarely) then you do indeed want it to be automatically built on some kind of recurring basis so it’s ready to go when you need it. And the tooling makes it (relatively) easy to have that happen.
I used to update the internal version of numpy for Google and if people asked me to rollback after I made my update (having fixed all the test failures I could detect), and they didn't have a test, well, that's their problem. The one situation where that rule wouldn't apply is if I somehow managed to break production and we needed to do an emergency rollback.
I shed a tear when some of my old, unused code was autodeleted at Google, but nowadays my attitude is: HEAD of your version control should only contain things which are absolutely necessary from a functional selection perspective.
How do you encourage testing?
Most teams are pretty understanding when they break you on some untested code, and will work with you to fix it, but the very first step in working together is for you to write a test that shows the issue.
The resultant culture is heavily biased toward writing tests before something bad happens to you.
> why the code exists in the first place
If the code is unreachable it’s at best a “possibly will be used in the future” and most likely simply something that was used but not deleted when it’s last use was removed (or a YAGNI liability).
If you can find a piece of code included in build targets but unreachable in all of them, it’s typically safe to delete. And it’s not done without permission generally, automation will send the change to a team member to double check it’s ok to delete/nobody is going to start using it soon.
So if there's a fancy onLocusSwarmAttack() that has never been run outside a test in a module to help applications deal with various kinds of datacenter outages called TenPlagues, it won't be in the crosshairs of Sensenmann. But if one day that module gets a successor (perhaps BeiWeltuntergangScheibeEinschlagen if it's made by the Zurich team?) and all code is supposed to eventually migrate to the new one, Sensenmann will tell you that TenPlagues has finally left the building when (if) it has happened.
Again, just my impression from the article and the discussions here (many seem to think it's about onLocusSwarmAttack()), probably resulting from a linguistic barrier between the language used at Google to talk about their monorepo and more conventional English. I guess they have done some quite deliberate shifts to help people "think monorepo"?
I think the real logical flaw is that Fencers (as I will now call them) put the blame on the person who removes an apparently useless fence. But they're wrong. The real blame lies with the person who built the apparently useless fence and didn't put a sign on it explaining why it shouldn't be removed.
No, that would only be the case if one would never understand any code. Chesterton's Fence consists of two parts ("understanding some code" as a precondition to "removing some code"), and leaving one or the other part out makes it some other thing than what Chesterton's Fence means.
> The real blame lies with the person who built the apparently useless fence and didn't put a sign on it explaining why it shouldn't be removed.
Chesterton's Fence is not about blame, or the past in general - it is about how to deal with things that are in the present. (Although I agree that the original fence-builder should have left a note or two!)
I follow the principle when I remove code, and it’s a reason why good code comments are important. “Oh yeah, this was written for [x] which is no longer a thing, we can remove it now”
That's worth explaining: it's automated code deletion, but the owner of the code (a committer to that directory hierarchy) must approve it, so it's rare there's ever a false deletion.
Agree: Yes, you are correct, merely observing that a code path was never executed in the last 6 months is not the same as understanding why the code path was created in the first place. There might be the quite real possibility of an infrequent event that appears just once in every two years or so (of course, this should also be documented somewhere!).
Disagree: Pragmatically, we have an answer if the code path was not executed after 6 months use in production and test: We know that, with a very high probability, the code path was created either by mistake (human factor) or intentionally for some behavior that is no longer expected from our software. To continue the Fence metaphor, regarding Sensenmann: After 6 months, we know about the Fence that 1) it has no role to play in keeping the stuff out that we want out (that was all done by other fences that were had contact with an animal at least once) and 2) that it might have been used to keep out flying elephants or whatever, but no such being was observed in the last 6 months (at least the fence made no contact with it, which it then should have!) and probably went away.
That said, having a human in the loop is probably a good idea.
If a chunk of code isn't actually deployed somewhere, mark it as a candidate for culling.
Probably requires some kind of metadata provenance for deployed artifacts.
For Java, I thought Maven had a stock manifest.mf entry for the source repo. Alas, a quick search only reveals that the archiver plugin has an entry for the project's "url".
https://maven.apache.org/shared-archives/maven-archiver-2.5/...
Which for in-house projects is probably sufficient.
This is more (or less?) the same as industry best practices, just scaled up. There is a challenge in scaling up, as there is more potential for someone to mess it up. But it's the same technique.
So what am I missing?
That's like saying S3 is the same as ext4, their the same, just scaled up! This is a poor argument, you'll note that S3 and ext4 are entirely different things, not "challenges", fundamentally different implementations.
Google is the only company I've ever worked for that automatically deleted dead code, let alone across a company of 100k+ SWE.
Our internal practice is to delete code if you suspect it's unused, run tests, and if it doesn't affect any tests, go for it. This could be automated, but it is not pressing enough, so we didn't automate it yet.
We could though, and it may even be a good idea, but I still don't get the novelty. But I appreciate your point of view.
This is one of the details that the blog post goes into. Sounds like it's not as trivial and obvious a problem as you think it is, and you would have benefited from just not dismissing the post because of that.
The specific topic here is not one of those google problems to me, as I can compare it to other problems we already solved. But yes, we could miss that critical point where a totally different problem domain emerges just from one order of magnitude more, so fair game.
No, unit testing was NOT introduced 20 years ago. As an example, Perl 1 was released about 35 years ago with a unit test suite that got run on every install. Every version of Perl has done so since, and since CPAN came along, most Perl modules have followed suit. This was the secret sauce behind Perl's reputation for being so portable.
Nor was Perl a pioneer. In fact unit testing was used in the 1960s on the Apollo program, and was even called unit testing. I believe that the concept can be dated back to a 1950s textbook but I can't find the reference.
So unit testing is over 60 years old.
The whole point of these comment sections is to have a discussion. If the point of this site was just to read articles there wouldn't be a comment section.
Dead code path detection is an interesting topic which I think the linked article doesn’t address at all.
And with enough samples we can learn what code paths are being taken. You can have your dead code bot look at the number of samples that have been collected to know if it should be enabled for something. For example if you have 6 mounths worth of samples of a piece of software running on 10k machines you will get a good idea on what code isn't being used.
>Anything that inspects the runtime (dynamic) behavior of code isn't good enough for a code deletion tool.
I disagree since the code that is being run is by definitions not dead code.
It sounds like the same issue exists with this approach of removing unused libraries and binaries that have not been run in a while. My suggestion is just expanding it from binaries that haven't run to libraries that haven't run. You could even just put this information into some code health dashboard. If code isn't run that means that it isn't providing value or it isn't being tested in production. How can you be sure some rare failure case actually works and doesn't take down the system if you never test it out to see if it works.
>GWP is also only able to instrument a very small number of requests.
As I mentioned before this could be limited to popular services where it is enabled and where a small number of a giant number of requests is still a big number.
Not really, it's the difference between knowing that something isn't plugged in and hoping that it isn't powered on.
Libraries that aren't run are deleted, you're suggesting to expand this to stochastically unexercised code paths within libraries that are run. (as opposed to provably unexcercisable code paths, which I think there are tools that do do this but are opt-in instead of on-by-default).
> If code isn't run that means that it isn't providing value or it isn't being tested in production.
Or can't show up in profiling for any number of reasons (I'm immediately brought to the idea of a CHECK fail which would not every show up in profiling I don't think, but could be exercised regularly, and if is included intentionally is likely preventing some kind of data corruption issue so removing it would be terrible).