Page MenuHomePhabricator

[Utils] Clean another duplicated util method.

Authored by asbirlea on May 31 2019, 2:21 PM.



Following the cleanup in D48202, method foldBlockIntoPredecessor has the
same behavior. Replace its uses with MergeBlockIntoPredecessor.
Remove foldBlockIntoPredecessor.

Diff Detail


Event Timeline

asbirlea created this revision.May 31 2019, 2:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 31 2019, 2:21 PM

Looks like a nice cleanup to me.

Why not remove foldBlockIntoPredecessor here?

asbirlea updated this revision to Diff 202797.Jun 3 2019, 2:03 PM

No reason for not removing the unused method.
Squashed a local NFC commit I had into this one.

asbirlea edited the summary of this revision. (Show Details)Jun 3 2019, 2:03 PM
dmgreen accepted this revision.Jun 3 2019, 2:34 PM


This revision is now accepted and ready to land.Jun 3 2019, 2:34 PM
This revision was automatically updated to reflect the committed changes.

This caused a serious compile time regression. used to take 75 s to compile, now it takes 220 s. Repro with clang -target x86_64-w64-mingw32 -c -O3 glew-preproc.c.

It appears the difference is in the way the DT updates are done.
+@kuhar, @NutshellySima: Could you please take a look at the way the updates are done in MergeBlockIntoPredecessor vs (the removed) foldBlockIntoPredecessor? I tried to use a Lazy strategy and flush after doing all the potential merges, but the regression is still there. Can we get the same performance with the DomTreeUpdater here?

Following up: I can match the previous performance if all update inserts are done first. I'll send a patch for this. But perhaps this should be done inside DTU (i.e. sort updates)?