A quiet refactor that paid for itself in a week

This is a small story about a small refactor. I am writing it down because the most useful engineering I have done lately has been this shape, and I never see it written about. Blogs are for the dramatic stuff. Most of the work is not dramatic.

The smell

There was a class in our Nexus codebase called ArtifactRequestContext. It started life, years before I arrived, as a tidy little helper that held the three or four things a request handler needed: the caller's identity, the target repository, a correlation ID. A request would come in, we would build one of these, pass it through the handler chain, and throw it away at the end. Fine.

By the time I was looking at it, ArtifactRequestContext had twenty-seven fields. It held the caller identity. It also held a lazily-resolved upstream-proxy client. It held a mutable buffer used for streaming responses. It held a flag called skipSecurityChecks that was set to true in exactly one code path and, I eventually discovered, quietly read in four others.

I did not go looking for this. I was chasing an unrelated p99 regression on the resolution read path. The flame graph kept pointing me at the constructor of this class, which was doing far more work than its name suggested. The name said context. The body said: initialize, resolve, allocate, and carry a decade of accreted decisions.

That is the first smell. When a class named for one thing has to do six things to exist, the code is telling you something.

The measurement that made the decision defensible

Before I touched it, I wanted a number I could point to. You cannot refactor an important class on vibes.

I added a small bit of instrumentation around the constructor — elapsed time, allocation count, a histogram keyed on the request path — and ran it for two days in a staging tenant that mirrored one of our noisier customers. What came back was that the average request was spending seven to twelve milliseconds inside this constructor, on the hot path, for work that the request did not always need. Roughly 60% of request flows never touched the lazily-resolved upstream client, but every one of them was paying for its setup. The streaming buffer was being allocated up front even for metadata-only responses, which was most of them.

Seven to twelve milliseconds per request is not an emergency. It is a tax. Stack enough taxes and your p99 starts to drift, the way ours had.

I wrote a two-paragraph doc. Here is the cost. Here is a sketch of the shape I want instead. Here is what I am not proposing to change yet, and why. I sent it to two engineers I trust, one of whom had touched this class more than anyone, and asked them to poke at the shape.

They both pointed out the same thing, which is why you show work to people who know it better than you do: the skipSecurityChecks flag could not just move. It was load-bearing in a way that was not documented, and had a specific contract with an auth middleware I had forgotten about. A clean refactor that did not respect that contract would be a correctness regression, not a performance win.

The PR shape

The PR that landed was smaller than the first one I drafted. I had wanted to split ArtifactRequestContext into five small objects, each focused. That would have been the textbook answer. It would also have been a three-week project with a migration across every handler in the codebase.

Instead I did this:

1. Kept ArtifactRequestContext as the external shape. Every caller continued to get the same type back.
2. Inside it, moved the upstream-client initialization behind a Lazy<> so it only resolved on first access.
3. Moved the streaming buffer allocation out of the constructor and into the one handler that needed it, which was two lines of code.
4. Left skipSecurityChecks exactly where it was. Added a comment explaining the auth-middleware contract, because I had just learned it the hard way and did not want the next person to.

Four files touched. About 120 lines changed. Two review rounds, both useful, neither painful.

I want to call out the decision to not split the class. Every instinct I had said I should. A class with twenty-seven fields is a code smell. But the refactor I actually shipped was the one that addressed the measured problem and left the rest of the code alone. The split was a future project, if it ever became worth doing at all. Most of the time, it will not.

What the metrics showed in a week

I shipped it behind a feature flag for one tenant, watched for a day, rolled it to the next ten, watched for two more, rolled it broadly.

Inside a week, the average cost of constructing a request context went from around nine milliseconds to just under two. P99 on the specific read path I had originally been investigating moved from 340 ms to 210 ms. That was more than the constructor fix could account for on its own, which meant there was another cause I had not found yet — but the constructor fix had made the second problem visible, which was part of the point.

Allocation pressure on the service dropped by a measurable amount — small but not tiny. JVM GC pause times at the p95 came in about 15% lower the following week, which was a thing I had not predicted. The reviewer who cared most about GC noticed it before I did, which was pleasant.

Nobody wrote a release note. It was a chore commit with a short message. The next week, I moved on to the second cause of that p99 problem, which turned out to be in a completely different layer of the stack.

What I take from it

A few things, none of them surprising.

Measurement before the refactor. Not because I did not trust my read of the code — I did — but because the number was what let me explain the decision to someone who had not read the code. A two-paragraph doc with a measured cost in it is harder to argue against than an opinion.

The refactor I shipped was smaller than the refactor I wanted to ship. That gap is where most of the growth has been for me lately. The bigger version would have been more satisfying to write and much harder to justify to the team. Smaller was the right call, even though it left visible mess in the code. Visible mess is not the enemy. Invisible mess is.

And the thing I keep learning: the moment you add a measurement, you make a second problem findable. Almost every small refactor I have done in the last two years has surfaced a neighboring issue that I did not know existed. That is not a failure of the refactor. That is the refactor doing its secondary job, which is to clear enough underbrush that the next thing becomes visible.

This one paid for itself in a week. Most of them do not pay back that fast, and I am fine with it when they do not. The discipline is to keep making small, measurable, boring improvements, the way you would pay down a credit card — one line at a time, without congratulating yourself for it.