@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Clean up some ordering and strata edge cases in Phrequent

Summary:
Ref T3569. Two issues:

# Since `sort()` is not stable, instantaneous events (ending on the same second they start) would sometime sort wrong and produce the wrong results. Guarantee they sort correctly.
# Because events can end at any time, there are some additional special cases the algorithm didn't handle properly. Draw a bunch of ASCII art diagrams so these cases work properly.

Test Plan:
- No more fatal when tracking an object for the first time.
- Unit tests.

Reviewers: btrahan

Reviewed By: btrahan

CC: skyronic, aran

Maniphest Tasks: T3569

Differential Revision: https://secure.phabricator.com/D7350

+220 -9
+121 -8
src/applications/phrequent/storage/PhrequentTimeBlock.php
··· 31 31 32 32 $timeline = array(); 33 33 $timeline[] = array( 34 + 'event' => $event, 34 35 'at' => $event->getDateStarted(), 35 36 'type' => 'start', 36 37 ); 37 38 $timeline[] = array( 39 + 'event' => $event, 38 40 'at' => nonempty($event->getDateEnded(), $now), 39 41 'type' => 'end', 40 42 ); ··· 46 48 foreach ($preempts as $preempt) { 47 49 $same_object = ($preempt->getObjectPHID() == $base_phid); 48 50 $timeline[] = array( 51 + 'event' => $preempt, 49 52 'at' => $preempt->getDateStarted(), 50 53 'type' => $same_object ? 'start' : 'push', 51 54 ); 52 55 $timeline[] = array( 56 + 'event' => $preempt, 53 57 'at' => nonempty($preempt->getDateEnded(), $now), 54 58 'type' => $same_object ? 'end' : 'pop', 55 59 ); ··· 58 62 // Now, figure out how much time was actually spent working on the 59 63 // object. 60 64 61 - $timeline = isort($timeline, 'at'); 65 + usort($timeline, array(__CLASS__, 'sortTimeline')); 62 66 63 67 $stack = array(); 64 68 $depth = null; 65 69 70 + // NOTE: "Strata" track the separate layers between each event tracking 71 + // the object we care about. Events might look like this: 72 + // 73 + // |xxxxxxxxxxxxxxxxx| 74 + // |yyyyyyy| 75 + // |xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx| 76 + // 9AM 5PM 77 + // 78 + // ...where we care about event "x". When "y" is popped, that shouldn't 79 + // pop the top stack -- we need to pop the stack a level down. Each 80 + // event tracking "x" creates a new stratum, and we keep track of where 81 + // timeline events are among the strata in order to keep stack depths 82 + // straight. 83 + 84 + $stratum = null; 85 + $strata = array(); 86 + 87 + 66 88 $ranges = array(); 67 89 foreach ($timeline as $timeline_event) { 68 - switch ($timeline_event['type']) { 90 + $id = $timeline_event['event']->getID(); 91 + $type = $timeline_event['type']; 92 + 93 + switch ($type) { 69 94 case 'start': 70 95 $stack[] = $depth; 71 96 $depth = 0; 97 + $stratum = count($stack); 98 + $strata[$id] = $stratum; 72 99 $range_start = $timeline_event['at']; 73 100 break; 74 101 case 'end': 75 - if ($depth == 0) { 76 - $ranges[] = array($range_start, $timeline_event['at']); 102 + if ($strata[$id] == $stratum) { 103 + if ($depth == 0) { 104 + $ranges[] = array($range_start, $timeline_event['at']); 105 + $depth = array_pop($stack); 106 + } else { 107 + // Here, we've prematurely ended the current stratum. Merge all 108 + // the higher strata into it. This looks like this: 109 + // 110 + // V 111 + // V 112 + // |zzzzzzzz| 113 + // |xxxxx| 114 + // |yyyyyyyyyyyyy| 115 + // |xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx| 116 + 117 + $depth = array_pop($stack) + $depth; 118 + } 119 + } else { 120 + // Here, we've prematurely ended a deeper stratum. Merge higher 121 + // stata. This looks like this: 122 + // 123 + // V 124 + // V 125 + // |aaaaaaa| 126 + // |xxxxxxxxxxxxxxxxxxx| 127 + // |zzzzzzzzzzzzz| 128 + // |xxxxxxx| 129 + // |yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy| 130 + // |xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx| 131 + 132 + $extra = $stack[$strata[$id]]; 133 + unset($stack[$strata[$id] - 1]); 134 + $stack = array_values($stack); 135 + $stack[$strata[$id] - 1] += $extra; 77 136 } 78 - $depth = array_pop($stack); 137 + 138 + // Regardless of how we got here, we need to merge down any higher 139 + // strata. 140 + $target = $strata[$id]; 141 + foreach ($strata as $strata_id => $id_stratum) { 142 + if ($id_stratum >= $target) { 143 + $strata[$strata_id]--; 144 + } 145 + } 146 + $stratum = count($stack); 147 + 148 + unset($strata[$id]); 79 149 break; 80 150 case 'push': 151 + $strata[$id] = $stratum; 81 152 if ($depth == 0) { 82 153 $ranges[] = array($range_start, $timeline_event['at']); 83 154 } 84 155 $depth++; 85 156 break; 86 157 case 'pop': 87 - $depth--; 88 - if ($depth == 0) { 89 - $range_start = $timeline_event['at']; 158 + if ($strata[$id] == $stratum) { 159 + $depth--; 160 + if ($depth == 0) { 161 + $range_start = $timeline_event['at']; 162 + } 163 + } else { 164 + $stack[$strata[$id]]--; 90 165 } 166 + unset($strata[$id]); 91 167 break; 92 168 } 93 169 } ··· 150 226 $result[] = $current; 151 227 152 228 return $result; 229 + } 230 + 231 + 232 + /** 233 + * Sort events in timeline order. Notably, for events which occur on the same 234 + * second, we want to process end events after start events. 235 + */ 236 + public static function sortTimeline(array $u, array $v) { 237 + // If these events occur at different times, ordering is obvious. 238 + if ($u['at'] != $v['at']) { 239 + return ($u['at'] < $v['at']) ? -1 : 1; 240 + } 241 + 242 + $u_end = ($u['type'] == 'end' || $u['type'] == 'pop'); 243 + $v_end = ($v['type'] == 'end' || $v['type'] == 'pop'); 244 + 245 + $u_id = $u['event']->getID(); 246 + $v_id = $v['event']->getID(); 247 + 248 + if ($u_end == $v_end) { 249 + // These are both start events or both end events. Sort them by ID. 250 + if (!$u_end) { 251 + return ($u_id < $v_id) ? -1 : 1; 252 + } else { 253 + return ($u_id < $v_id) ? 1 : -1; 254 + } 255 + } else { 256 + // Sort them (start, end) if they're the same event, and (end, start) 257 + // otherwise. 258 + if ($u_id == $v_id) { 259 + return $v_end ? -1 : 1; 260 + } else { 261 + return $v_end ? 1 : -1; 262 + } 263 + } 264 + 265 + return 0; 153 266 } 154 267 155 268 }
+99 -1
src/applications/phrequent/storage/__tests__/PhrequentTimeBlockTestCase.php
··· 99 99 ), 100 100 $ranges); 101 101 102 - 103 102 $event = $this->newEvent('T2', 100, 300); 104 103 $event->attachPreemptingEvents( 105 104 array( ··· 118 117 $ranges); 119 118 } 120 119 120 + public function testTimelineSort() { 121 + $e1 = $this->newEvent('X1', 1, 1)->setID(1); 122 + 123 + $in = array( 124 + array( 125 + 'event' => $e1, 126 + 'at' => 1, 127 + 'type' => 'start', 128 + ), 129 + array( 130 + 'event' => $e1, 131 + 'at' => 1, 132 + 'type' => 'end', 133 + ), 134 + ); 135 + 136 + usort($in, array('PhrequentTimeBlock', 'sortTimeline')); 137 + 138 + $this->assertEqual( 139 + array( 140 + 'start', 141 + 'end', 142 + ), 143 + ipull($in, 'type')); 144 + } 145 + 146 + public function testInstantaneousEvent() { 147 + 148 + $event = $this->newEvent('T1', 8, 8); 149 + $event->attachPreemptingEvents(array()); 150 + 151 + $block = new PhrequentTimeBlock(array($event)); 152 + 153 + $ranges = $block->getObjectTimeRanges(1800); 154 + $this->assertEqual( 155 + array( 156 + 'T1' => array( 157 + array(8, 8), 158 + ), 159 + ), 160 + $ranges); 161 + } 162 + 163 + public function testPopAcrossStrata() { 164 + 165 + $event = $this->newEvent('T1', 1, 1000); 166 + $event->attachPreemptingEvents( 167 + array( 168 + $this->newEvent('T2', 100, 300), 169 + $this->newEvent('T1', 200, 400), 170 + $this->newEvent('T3', 250, 275), 171 + )); 172 + 173 + $block = new PhrequentTimeBlock(array($event)); 174 + 175 + $ranges = $block->getObjectTimeRanges(1000); 176 + 177 + $this->assertEqual( 178 + array( 179 + 'T1' => array( 180 + array(1, 100), 181 + array(200, 250), 182 + array(275, 1000), 183 + ), 184 + ), 185 + $ranges); 186 + } 187 + 188 + public function testEndDeeperStratum() { 189 + $event = $this->newEvent('T1', 1, 1000); 190 + $event->attachPreemptingEvents( 191 + array( 192 + $this->newEvent('T2', 100, 900), 193 + $this->newEvent('T1', 200, 400), 194 + $this->newEvent('T3', 300, 800), 195 + $this->newEvent('T1', 350, 600), 196 + $this->newEvent('T4', 380, 390), 197 + )); 198 + 199 + $block = new PhrequentTimeBlock(array($event)); 200 + 201 + $ranges = $block->getObjectTimeRanges(1000); 202 + 203 + $this->assertEqual( 204 + array( 205 + 'T1' => array( 206 + array(1, 100), 207 + array(200, 300), 208 + array(350, 380), 209 + array(390, 600), 210 + array(900, 1000), 211 + ), 212 + ), 213 + $ranges); 214 + } 215 + 121 216 private function newEvent($object_phid, $start_time, $end_time) { 217 + static $id = 0; 218 + 122 219 return id(new PhrequentUserTime()) 220 + ->setID(++$id) 123 221 ->setObjectPHID($object_phid) 124 222 ->setDateStarted($start_time) 125 223 ->setDateEnded($end_time);