With large patches, check-patch-relevance can get stuck. Part of this is because of the constant re-computation of regexes, but it's always possible for extremely large patches to jam up EWS. Along with making check-patch-relevance more efficient, we should provide a timeout.
<rdar://problem/77327168>
Created attachment 440094 [details] Patch
Comment on attachment 440094 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440094&action=review > Tools/CISupport/ews-build/steps.py:467 > for change in patch.splitlines(): You can consider adding a check here if the line is a filename or not. We actually need to match only the filenames of file being modified in the patch, not the complete patch content. e.g.: if not (change.startswith(b'---') or change.startswith(b'+++')): continue
Also, can you please profile the relevant code portion before and after your change (maybe on your local computer), especially on attachment 440066 [details] and attachment 427346 [details].
Created attachment 440105 [details] Patch
(In reply to Aakash Jain from comment #4) > Also, can you please profile the relevant code portion before and after your > change (maybe on your local computer), especially on attachment 440066 [details] > [details] and attachment 427346 [details]. Did some profiling. The kicker is really line size. If we limit line size to 250, the code in the attached patch takes less than a half-second.
Comment on attachment 440105 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440105&action=review > Tools/CISupport/ews-build/steps.py:399 > + bindings_regexes = [ I think that the new name doesn't describe the variable well. Either bindings_paths or bindings_path_regexes would be better in my opinion. Same comment for all the others below. > Tools/CISupport/ews-build/steps.py:471 > + if regex.search(change[:250]): [:250] doesn't appear to be explained, and I don't think that it's even accurate to only look at the first 250 characters (.*jsc.* could be anywhere).
(In reply to Alexey Proskuryakov from comment #7) > Comment on attachment 440105 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=440105&action=review > > > Tools/CISupport/ews-build/steps.py:399 > > + bindings_regexes = [ > > I think that the new name doesn't describe the variable well. Either > bindings_paths or bindings_path_regexes would be better in my opinion. > > Same comment for all the others below. Will change the variable names. > > > Tools/CISupport/ews-build/steps.py:471 > > + if regex.search(change[:250]): > > [:250] doesn't appear to be explained, and I don't think that it's even > accurate to only look at the first 250 characters (.*jsc.* could be > anywhere). I'll add this to the changelog, but it's the really bit lines that end up being what kills our performance. It's true that we're potentially sacrificing some correctness. But it's also unlikely that a patch will have a line that exceeds 250 characters and contains a legitimate match in truncated portion of the line, a match which does not appear in another line in the patch.
Created attachment 440124 [details] Patch
Committed r283528 (242495@main): <https://commits.webkit.org/242495@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 440124 [details].
<rdar://problem/83864363>
Confirmed that EWS can now process similar patch efficiently. e.g.: https://ews-build.webkit.org/#/builders/1/builds/50144 took only 1 second in check-patch-relevance. Earlier it was taking 30+ minutes in similar patch in https://ews-build.webkit.org/#/builders/1/builds/50001