Archive for the ‘Coding’ Category

The Ancient Code of a Million Developers

Thursday, October 11th, 2007

cubeLifeView.gif

Today I had to spend a lot of time on a application that got dumped on my lap after the last developer working on it couldn't get a new price feed hooked into it and was asked to leave. Since I had done a lot of work on this price feed, it seemed natural that I get the app hooked up to the new feed. It took me a few days and worked great. Then there came the need to add in permissioning for fee-liable exchange data, so again I was the logical choice because I had done similar work for other apps. Ahh... that slippery slope.

It didn't take long for me to be the maintainer of this app. That typically meant that features that haven't worked in years suddenly working because I found mistakes in the code and/or database and then they magically started working again. Some I was specifically asked to fix, others were resurrected completely by accident when I was fixing something else.

Now I've been working on this code for about 9 months. Not full-time, more on an as-needed basis, so I don't mind too much. But I need to say that this code is a real mess. I mean the worst code I've ever seen.

It's primarily C++ on Windows using Visual C++ - but the build systems is GNU make on Cygwin. It uses JNI, so the JDK is involved and that's a mess. There is virtually no documentation, there are dozens of sub-projects. The database access is through a separate process on another machine that has it's own configuration, it's asynchronous and not easily followed. It's over-designed and poorly coded. In short, it's a mess, and it'd be easier to re-write it but the logic is so buried in the code that it'd be easier to sit down and talk to the users to see what it's supposed to do as opposed to how it does it.

One of the features of this app is the ability to create new views (browsers) on the data. This controls the columns shown in the table, their order, how the view is updated, etc. Yesterday one of the big users of the app asked me if it would be possible to make it so that you could create new browsers. The app looks like it's supposed to work, but it doesn't. So I said I'd look into it and see what I could find out. Yikes! what a mess.

The code was a mess and had been that way since 1999 - I found comments where they had hard-coded the name of the new browser to 'x' - literally an 'x', and it was commented as happening in 1999. What were they thinking? I have no idea. I tore that out, figured out what was being passed into the method by way of these odd notification objects, and realized that it wasn't that far from working - on the client side. It took less than 50 lines of code changed, but finding those lines was the challenge. The real problems were on the database layer configuration, and stored procedures.

When moving to Sybase 12.5 the IDENTITY column had to change from an 'int' to a 'number', and in doing that, most all of their stored procedures were broken and never fixed. The access layer expected ints, and they weren't being returned that way - so basically, all the database work was getting dropped in the bit bucket. It was similar to what I'd seen before in this app, but what a striking example of poor updates and testing.

Then there's the bad UI, but that's just because it was created by so many developers over so may years there's no way it was going to have a consistent vision and style. Horrible. Which brings me to the point of this post - the code is ancient. It's been worked on by so many developers that had no idea what the point and architecture of the code was originally about. It's been slapped, hacked, poorly tested and given unworkable hard-coded hacks. It's amazing it still compiles. But this is what some people call developing. I can't imagine putting my name on this if I'd done it. I'm happy with the fixes I've made, but those are only in light of how bad it was to start with.

It's just amazing it works at all.

Historical MarketData Source and CKString Fun

Wednesday, October 10th, 2007

xcode.jpg

Yesterday afternoon one of the developers came by to ask me about finding historical prices for delisted symbols. "That's hard", I said - because it is. Bloomberg has historical data back some 20 years, but you have to know their symbology to get it out of Bloomberg, and one of the things my MarketData server does is to allow us to use our own symbology and it converts this to the symbology of the data provider (in this case, Bloomberg) and then requests the data. This is great so long as you can do the symbology mappings. The problem comes in with delisted symbols - they aren't in our mappings table so I can't convert from our symbology to Bloomberg's. This means that eventhough you might be able to get at the data, you don't know what to ask for.

I told him he'd have to investigate another data provider and get the data from them. This other provider needs to have the historical data and some sense of the re-use of tickers so that given a date and a ticker it knows that there was one - and only one instrument for that ticker on that date. This kind of historical data is not common, but it's available. You just have to know where to look.

So we started looking.

There's a project in the Bank that handles a ton of historical data from a lot of sources - one is Bloomberg. So we did a few tests to see if this source - which was in my MarketData server as a provider, had the data for a delisted company. Turns out it did. But at the same time, I found that there were problems with how I was calling and using this data source. So I dug into fixing them.

The first was pretty easy - when I requested data of this source, if I received nothing, then I assumed it was an error. In reality, if there's no data, they returned nothing. So if you asked for a delisted symbol after it was delisted, you'd receive nothing. I fixed those tests up to allow for nothing to be returned. Easy enough.

The second one was more interesting. The data fields you request should stay the way you request them - case-wise, but I wasn't preserving the case of some requests and mismatching the case of others. So I added several nice little methods to CKString to allow the user to copy a string and uppercase it (the only existing method uppercased the string in-place) and then I added equalsIgnoreCase() like Java's String to allow the code to not have to worry as much about case in the tests. When I put these in and realized that I needed an uppercase copy of the fields for getting data from the source, caching it, and then using the original field list to pull it out of the cache, things worked beautifully.

In the end, it was a lot of fun to add these things and see how they simplified the code I was trying to write to fix the problem. Lesson learned: add power to the underlying libraries and that will make any higher-level fixes much easier.

Looking at both Sides of Java and Web Browsers

Tuesday, October 9th, 2007

java-logo-thumb.png

OK, I'm trying to be fair and even-handed about this, but this morning I got another gotcha from Java (1.6.0_02) and IE (6) and Firefox (2.0.0.7). It's this thin client development that is 90% of what's needed but when you hit that 10% it's a pain in the rear to try and get around it. This morning I spent several hours doing just that.

The problem came up because one of my servers uses a web client to maintain it's internal data. This is nice in that the previous 'editor' was a Java application and it was a lot heavier than the support staff needed to have. Plus, with the web delivery, it was easier to have these folks support the server from London and home. But there's always a cost.

One of the things I did a while back was to have the users able to edit start-of-day (SOD) positions for the instruments in the master file. In order to do this in a reasonably useful way, it meant that I had to have a Java applet. This was not the first Java applet on the editor, and I knew that there would be IE/Firefox issues and how to get around them to make sure that the applet launched, etc. Nothing new there. What was new was the fact that I needed to send back data when the user hit 'Save'. I'd probably have been happy enough with Java applets if I hadn't had to do that. But I did.

Moreover, I had to send these updates through CGI scripts where the old and new data were arguments to the GET request. Not terribly hard, but when you find that IE still allows JDK 1.6.0_02 to send URLs to a server and get back answers, and Firefox doesn't, then we get into the lovely realm of applet signing.

It's not that terribly hard, but I didn't want to get a certificate that would mean the users would have to manually accept, so I went on a search for the location of the certificate that the web developers have built to enable them to sign their applets. This way, I know the certificate is allowed on all the boxes, and I don't have to worry about that.

Easier said than done, and honestly, it wasn't Java's fault. But the idea that in these days of corporate intranets and apps deployed on them, it would seem reasonable to have the security system say "Hey, if it's on this domain, it's OK". I know that's not necessarily safe, but coupled with the fact that there's security in the assignment of IP addresses and the domain naming, it's very unlikely that someone is going to put a box on the net with the right IP and domain to publish fake jars. Could happen, but it's not likely.

So I had to sign the applet. Interestingly enough, Googling this revealed that it wasn't until a recent release of 1.6.0 that this difference in behavior between IE and Firefox came to light. Previously, the behavior of IE and Firefox was the same in that it'd allow the URL connections from the applet to a machine not hosting the applet. So this might change again in the future. Yum...

The next problem was NFS... and it's likely a linux problem, or a difference in the NFS system we have in the Shop, but it turns out that if you copy a file from one machine to a shared filesystem, there's about a 70% chance that the web server will think this is a corrupted file. So, after seeing that the ClassNotFound exception was on the Java console, I went to the web server and copied the file to the same shared location. This finally worked.

After all this, things started to click and I got all the issues worked out.

Now I know that Java isn't perfect, and it's a lot better than ActiveX, which came before it, and I'll even say that it's a alot easier to deal with than AJAX, but I keep thinking that there has to be a better way. I know that AJAX is making strides, but it's not really a revolution, more an evolution. I'm wondering where's the revolution in thin clients? Where's the easier system of making clients deployable easily and run cleanly. I've seen Java WebStart and its a lot like Marimba - you download the app and run it. Yeah, it makes deployment easier, but it's no different than fat clients.

Throwing together a Java RMI server would be possible, have the applet connect in and send the updates - that'd get rid of the CGI scripts, but it's not going to make the security issues with Java any more manageable. Make it easier to write powerful apps that are easily deployed remotely. Tons of OSs have done it - X, NeXTSTEP, even VNC does this. But these are for something large, like a desktop or a big app. I'm thinking small - like most applet work. That would be nice to find.

Debugging Replicated Database Problems

Thursday, October 4th, 2007

database.jpg

Well... as I thought it might, the read-only copy of the instrument master database failed on me last night and while the primary is working fine, I feel it's necessary to be able to find a test case, or condition, where the replicated database fails so that I can give this to the team working on that project and they, in turn, can fix the underlying issue(s). I'm sure the local database admins will be be involved, as they have to be as we don't have that level of control over the servers and the machines. So, mauled by the sharks (from my previous post) I go back into the water trying to find the test case that will highlight the problem.

Last evening, the server was restarted at 5:49 pm, and the symbol set was divided into four groups of 889 underlyings and all four were sent out to the database proxy for loading. Typically, all four will finish within a few minutes of each other, but last night the first one finished at 17:55:12 and the second finished at 17:55:50 - but the third and fourth never finished. When I reconfigured the server to point to the primary, the four finished within 4 mins of each other - as they should. Clearly, there was something with the replicated database that was causing two of the loading threads to sit there waiting for data to come back. The question is, how to reproduce this?

It gets more of a quandary when you take into account that my development server started at 7:00 pm local time and it was fine using the read-only database - all four of it's loading threads finishing within a few minutes of each other. So there's something that's happening to the replicated copy between 5:50 and 7:00 pm that caused this problem, but it was gone by 7:00 pm.

I have a simple web page on the server's editor that allows me to look at the database operations that are being done in the code to see what the data is in the database and what's being retrieved. This has really helped a lot in the diagnosis of database issues like bad prices and missing key values. Yesterday, when we were having problems with the replication and the prices, I did have a few times when this page would not return all the data. Because it's a Perl script, it'd return what it had processed, but it would still act as if there were more to read (because there was), and yet nothing would come back. I'd love to be able to reproduce that for the guys.

Unfortunately, I haven't been able to. I have no tools at my disposal other than the requests I make. I'll keep hitting it throughout the day, but I don't hold out a lot of hope that this is going to point to anything conclusive. This leads me back to the same spot I was at yesterday - do I trust it? Today, however, the answer is different: No. I'll trust it when I have to trust it and not before. Since no one is really as concerned about this as I am, I'll stick to using the primary and see where the chips fall. There's no reason to risk production outages when all I've got to diagnose the problem is a few data loading scripts.

Interesting Bug in the Server

Tuesday, October 2nd, 2007

servers.jpg

This morning I got an email from one of the Hong Kong users about problems they were having with the theoretical values on OTC options in the server. While I didn't really get the proper picture from his email, the follow-up from a user in London provided the proper illumination to see what the problem was. Basically, when changing the volatility and/or dividends curve(s) for an instrument, the 'Open' greeks would be calculated properly and the 'Last' would not.

I dug into the code and saw that the 'Open' greeks are calculated no matter what, but the 'Last' are calculated only if they aren't in the cached data for that instrument already. This was the key - that the cached data was all wrong because it was generated based on the old curves, and with the new curves, all the cached data needed to be invalidated and recalculated.

Once I knew what I needed to do, it was just a matter of putting the code in-place to allow me to clear out the cache at the instrument level, then the underlying which would clear out all the derivatives' caches, etc. Then I had to put in the code to detect the change in the curves as read in from the editor, and putting it all together was pretty easy.

Once I had the code in-place, it was very easy to test and see that we are indeed getting the right greeks for changes in the curves. Also, I fixed a few issues in the editor so that it would not send the curves back to the server on an edit unless they had been edited by the user. This is going to help in simple efficiency as well as not making the server think things have changed when they haven't.

Added removeRows() to BKTable

Monday, October 1st, 2007

comboGraph.png

I had a new developer come up to me today and ask me if it were possible to remove a group of rows from a BKTable, and after a little bit of a sync on the terminology, it was clear that he wanted to delete a group of rows from the table based on some criteria. I was thinking about this and it seemed like a good idea even-though there's no way to do it now. So while he had to stick with iterating through the table and removing each one after testing it for it's applicability in the final results, I decided it was something I wanted to ask Jeff if he'd use it enough to add it to the class. Turns out, it's something that he'd like to see too. So I spent a little time today putting that into the BKTable.

I went with the idea that you'd provide this a JEP expression and it would either remove the rows that matched this expression or remove every row but those that matched this expression. Since I could use a lot of the same components that are in use in the filter table view this wasn't too hard. In fact, the effect is very similar to the filter table view, but in this case the removal is permanent and you can easily choose either set to remove.

After I got this in and tested I talked to Jeff and let him know that it'd be OK to have a list of a few things they wanted in BKit. He mentioned that his guys are suggested to come talk to me, with suggestions/problems so I guess there just aren't a lot of problems. OK.

Identity – The Often Overlooked Test Case

Tuesday, September 25th, 2007

Today I had a very interesting bug that has been sitting in the server's code base for ages. In one of the support libraries there's a standard C++ string class modeled loosely after the Java String class. I inherited this code and never gave it much thought - until today.

The original code had an operator=() method implemented like this:


    const jString & jString::operatorconst jString & anOther ) {
        empty();
        append(anOther);
        return *this;
    }

Where the empty() method and the append(const jString &) methods worked perfectly. The problem came into play when I had something like this in the code:


    jString     one("hello");
    
    one = one;

Now this does not make a lot of sense, and in truth, the actual code was a lot more convoluted than this - but the point is that it's setting itself equal to itself. In any case, it shouldn't have done what it did. What it did was to clear out the value and leave the string empty. Why? Because the empty() method cleared out the value of anOther so that the append() call had nothing to copy from. This was a pain in the rear because I believed that the operator=() was smarter than this. It wasn't.

The fix is simple:


    const jString & jString::operatorconst jString & anOther ) {
        if (this != & anOther) {
            empty();
            append(anOther);
        }
        return *this;
    }

and so long as we check to make sure we aren't operating on ourselves, we're good to go. I know that there are probably a good number of cases like this in the code and I'm going to have to check each of the operator=() methods in the libraries, but at least I know what to fix, and the fix is easy.

I know that I'm going to be paying a lot more attention to the overlooked case of 'self' in the code I write from now on.

Debugging is a Continuous Effort

Monday, September 24th, 2007

I was out on Friday feeling pretty crummy. Today I'm feeling a little less crummy only because I'm a little more used to it. When I got back, I saw that my analytics engine had been spitting out a bunch of NaNs for this one analytic. So I started to dig into the problem. What I found amazed me - it should have been broken much worse than this much earlier than this.

The engine is written in C++ and as such, those pesky NaNs are something you have to consider, and if you're clever, use them to indicate illegal values, etc. This engine has been running for years doing the same kinds of things day in and day out many times a day - seemingly without problem. But when I dug into it today I realized that a few months ago I changed the datasource for instrument prices and in doing that set the stage for a bug cascade that ended up biting me Friday while I was out.

The problem starts with the data source of prices - Reuters. When they are about 2 hours from a market open, they will zero out all the data in the records they send as an indicator that the instrument is about to enter the active market portion of the day. Normally, this is OK, but the problem comes in when you realize that you need to make a "price" from a bunch of zeros, and you realize that those zeros are telling you to use the historical mark for the instrument and not use the data from Reuters. I thought I had the code in the application to do that, but it seems I was more than a little mistaken.

No... I wasn't ignoring the zero prices, I was converting them to NaNs and putting them into the time series data. I know I was thinking that this would signal later in the code to skip this data point, but even that was unnecessary as I simply should not have overwritten good data with bad no matter what I was thinking I was going to do with it later.

After fixing that bug, I realized that even with the one data point a NaN, there was no reason for all the historical data points to also return NaN. As I looked into the problem more, I realized that I had made the historical calculations biased by the value for today. That way, once I've calculated the historical numbers, I subtract out the value for today and then when I call it again, I can simply compute the value for today, add it to the rest of the values and everything is up to date.

But again, since I was putting in a NaN for a zero price, I was messing up all the values by having the value for today so messed up. Amazing. Fixing it was not too bad - only took a little time, but it was the data change that started the whole problem. It's just amazing to me that I need to keep up with debugging when you change things like the data sources - based on the assumptions that come with the data.

Tricky Little Threading Bug

Thursday, September 20th, 2007

bug.gif

For the last two days I've been wrestling with a tricky little threading bug. The problem is in a class collection called SearchSpace and it's used for caching computed values that are computationally expensive to produce, so we cache them for interpolation of intermediate values. It's a standard 1D and 2D search space with linear and Taylor Series approximations built in. Most of the time everything runs fine, but two days ago I came into the office and saw that my dev server had a Seg Fault at the line:


    if (mHash[lIndex] == NULL) {
        ...
    }

The value mHash was NULL and that was causing the Seg Fault. The problem was, about 10 lines before this, I have code that looks like:


    if (mHash == NULL) {
        init(false);
    }

Given that the method this appears in is protected by a mutex for thread safety, one would think that it's impossible for the variable mHash to be NULL on entry, initialized, and then it's again NULL a few lines later. Or, if it's not NULL on entry, it's impossible for it to become NULL by the problem line. I know I was confused. But the impossible happens so often to me it's almost common.

The key to this problem is that the tHashMap (where this method exists) is getting deleted in the middle of the operation. There's no mutex issue to contend with, and on exit all the variables are destroyed, and I get a seg fault, so the question remained - who was doing the removal and why? Even more importantly, how to stop it without throwing a ton of locking and unlocking at the problem?

It turns out that the SearchSpace runs a purge() method every so often to clear out the unused data. Data that hasn't been accessed in 10 minutes is considered 'stale' or 'unnecessary', and is deleted. If we need to re-create it later, so be it, but we have to balance the memory footprint with the computational cost of generating the numbers. It turns out that 10 mins is a nice trade-off point for this application. When the purge() is being run, it looks for empty SearchSpaceNodes (which contain tHashMaps) and deletes them. This, then, is the culprit. If we're working on a nearly empty SearchSpaceNode and the purge() method deletes it, we're left with a dangling pointer and that's the problem. So how to fix it?

One of the things I've used successfully in the server is to implement a very simplistic retain/release counter. But given that there will be tens of thousands of these SearchSpaceNodes in the system, I didn't want to burden the system with a mutex and an integer for each node. There had to be a simpler way.

The answer was simple - add a simple bool ivar to the SearchSpaceNode called inUse. Have the addItem() method set it when it gets a SearchSpaceNode, do all the work it needs, and when it's done, it resets it. The purge() method then only deletes the empty SearchSpaceNodes that are 'not busy'. If it's busy one time through, and nothing is added, then the next time through purge() it'll be deleted. But now it's going to be impossible to get the seg fault due to the SearchSpaceNode being deleted out from under a working thread.

Not easy to see, and it could have been solved by using mutexes and making every method on the SearchSpaceNode thread-safe, but that's not completely necessary. I only need to protect this case, and with what I've got now, I've got the thread-safety I need and don't have the overhead of all those mutexes locking and unlocking all the time.

Coders that Get Angry at Code

Wednesday, September 12th, 2007

I was looking a few things up today and I could not help but hear a nearby coder getting angry at the code he was writing. Actually thinking that the code was disobeying him. I'm the first to admit that I've gotten mad while looking at code - most often at the guy who wrote the mess I was forced to fix, but to actually get mad at the code is something I find very funny.

I've always liked coding because it's about the only place in the world where what I asked for was done - without argument, exception, grief, or hassle. I mean really - people are far far more difficult to deal with than a machine. So in terms of frustration, coding is about as stress-free as things come. Oh sure, deadlines are a drag... trying to do more with less... there's a ton of ways to make any job a pain in the neck. But those are reasons to get irked regardless of the job.

It's like a carpenter getting mad at a hammer. It's a hammer - if you don't like what it's doing, hit something else. Don't get mad at the hammer. But I have worked around this particular coder for quite a while, and he's got talent, but he misses the boat when it comes to drive, commitment, and really enjoying the job.

I think he'd rather be somewhere else, but he took this job because it paid significantly better than his last job. I can't blame a guy for making a buck, but if that's the only reason you took the job, it's not going to be long before you realize you aren't really happy and you'll find another job.