Monday, October 5, 2009

There was some code was causing performance problems under certain circumstances. It was doing lots of XPath queries to build some data structures out of XML that was read from a file. So it was decided that those data structures would be cached in a map with the parameters that went into building the data structures being the key. Although I was not familiar with the code, it looked fine to me after reviewing what parameters went into building the data structures.

But it wasn't really fine. Fortunately, initial testing resulted in a bunch of ConcurrentModificationExceptions being thrown. The data structures were being modified after they were returned out of the cache. It could have been a lot worse. The modification of the data structures could have silently corrupted the cached data without being noticed in testing, and the code could have gone into production.

So the lesson is that when sharing mutable data structures that were previously newly constructed for each use, either redesign the data structures to be immutable, or make a new copy each time.

(However, what ended up being done was the code modifying the data structure was moved to being done before putting the data structure in the cache. The data structure remains mutable, which could be a problem down the line. All the comments and documentation in the world won't necessarily prevent someone from inadvertently adding code that changes an object from the cache.)

No comments:

Post a Comment