- Change Summary:
-
Updated based on the diff move change. We no longer yield the transformed opcodes.
- Description:
-
This change introduces some speed improvements for the diff viewer. Particularly in chunk and file listing generation.
The upcoming change for listing function/class names for collapsed regions adds a small amount of overhead to the diff viewer, which this change more than compensates for (in my tests, though it still needs testing with much larger files).
The primary changes include:
- Define ("New File") and ("New Change") only once, instead of per-file.
- Normalize line endings through a regex, instead of processing the entire file three times.
- Define the newline splitting regex once, instead of up to 4 times per file.
- Prevent the need for a temporary variable for interdiffs when swapping buffers.
- Yield the chunks being generated, instead of appending to a large array. It's up to the caller to decide how to handle this. Currently, we do turn this into an array before going into memcached, but it's more explicit and can provide memory savings in some potential uses down the road.
~ - Yield the transformed opcodes instead of stuffing them into a group. The upcoming move detection change may require that we go back to forming a group up-front, but it doesn't hurt for now.
~ - Store only changed chunk indexes, not the chunks themselves. This should offer memory and cache storage savings.
~ - Check the DiffData.modified dict directly, instead of going through a function. This offers a small savings, considering how often we do it. Furthermore, the new checks are only one statement, instead of two.
~ - Small performance improvement for fetching codes from a table. Using a try/except can be faster than using if and has_key.
~ - Bail earlier in some of the diff generation when we know we can, instead of first doing some computations and then throwing them away.
~ - Store only changed chunk indexes, not the chunks themselves. This should offer memory and cache storage savings.
~ - Check the DiffData.modified dict directly, instead of going through a function. This offers a small savings, considering how often we do it. Furthermore, the new checks are only one statement, instead of two.
~ - Small performance improvement for fetching codes from a table. Using a try/except can be faster than using if and has_key.
~ - Bail earlier in some of the diff generation when we know we can, instead of first doing some computations and then throwing them away.
~ - Move find_diagonal out into a class function. By declaring it within a function that's called often, we lose a lot of speed. Much was gained by doing this.
- - Move find_diagonal out into a class function. By declaring it within a function that's called often, we lose a lot of speed. Much was gained by doing this.
Speed up the diff viewer
Review Request #1299 — Created Dec. 16, 2009 and submitted
This change introduces some speed improvements for the diff viewer. Particularly in chunk and file listing generation.
The upcoming change for listing function/class names for collapsed regions adds a small amount of overhead to the diff viewer, which this change more than compensates for (in my tests, though it still needs testing with much larger files).
The primary changes include:
* Define _("New File") and _("New Change") only once, instead of per-file.
* Normalize line endings through a regex, instead of processing the entire file *three* times.
* Define the newline splitting regex once, instead of up to 4 times per file.
* Prevent the need for a temporary variable for interdiffs when swapping buffers.
* Yield the chunks being generated, instead of appending to a large array. It's up to the caller to decide how to handle this. Currently, we do turn this into an array before going into memcached, but it's more explicit and can provide memory savings in some potential uses down the road.
* Store only changed chunk indexes, not the chunks themselves. This should offer memory and cache storage savings.
* Check the DiffData.modified dict directly, instead of going through a function. This offers a small savings, considering how often we do it. Furthermore, the new checks are only one statement, instead of two.
* Small performance improvement for fetching codes from a table. Using a try/except can be faster than using if and has_key.
* Bail earlier in some of the diff generation when we know we can, instead of first doing some computations and then throwing them away.
* Move find_diagonal out into a class function. By declaring it within a function that's called often, we lose a lot of speed. Much was gained by doing this.
Tried this with several diffs. I didn't notice any diff generation issues.
Unit tests passed.
Took a diff applied to a ~750 line file and ran a generation test 5 times with the old code and 5 with the new code. On each test, I cleared the cache, restarted the server, and force-reloaded the page, measuring both the diff chunk generation times and the file info generation times using log_timer.
Results (in seconds)
Diff chunks Diff file generation
Old: 0.1668 1.2291
New: 0.1627 1.0686
---------------------------------------------
Savings: 0.0041 0.1605
(Note that chunk generation is part of the file generation and contributes to that time. Still, there's much we probably could do to improve the file generation.)
These are rough tests. Earlier when playing with this, I had some runs that showed savings of about 0.02 seconds on diff chunk generation, though that's not what today's tests are indicating. Still, the total savings due to the full file generation is pretty good.
