Moti Asayag has posted comments on this change. Change subject: <utils>: performance improvement ......................................................................
Patch Set 1: (6 comments) http://gerrit.ovirt.org/#/c/25940/1//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-03-14 13:28:42 +0100 Line 4: Commit: Martin Mucha <mmu...@redhat.com> Line 5: CommitDate: 2014-03-14 13:45:03 +0100 Line 6: Line 7: <utils>: performance improvement no need for the angle brackets, "utils: performance improvement" is fine Line 8: Line 9: * initialized ArrayList to avoid copying arrays (tradeof is Line 10: bigger memory footprint during initialization) Line 11: Line 16: this changes cuts CPU time approx to 50% of original. Line 17: --- Line 18: Note: code can be suboptimal when looking for LOT OF macs but range Line 19: offers to little of them: in that case unneccesarily big Line 20: array alocated and then trown away for no reason. But this should s/alocated/alhlocated Line 21: not be an issue. Line 22: Line 23: Change-Id: I09f87ebd1ea17d09a974ce6ca3e26bd2454a72fc Line 24: Bug-Url: https://bugzilla.redhat.com/1063064 http://gerrit.ovirt.org/#/c/25940/1/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java: Line 31: if (startNum > endNum) { Line 32: return Collections.emptyList(); Line 33: } Line 34: Line 35: List<String> macAddresses = new ArrayList<>(stopAfter); //initialize ArrayList for all potential records. (ignore that there need not be that many records. please add the comment above the line. Line 36: for (long i = startNum; i <= endNum; i++) { Line 37: if ((MAC_ADDRESS_MULTICAST_BIT & i) != 0) { Line 38: continue; Line 39: } Line 44: return macAddresses; Line 45: } Line 46: } Line 47: Line 48: return new ArrayList<>(macAddresses); //do a copy to reduce array length (array stored in ArrayList impl.) same here Line 49: } Line 50: Line 51: private static String macAddressIntToString(long macAddress) { Line 52: String value = String.format("%012x", macAddress); Line 49: } Line 50: Line 51: private static String macAddressIntToString(long macAddress) { Line 52: String value = String.format("%012x", macAddress); Line 53: // String original = StringUtils.join(value.split("(?<=\\G..)"), ':'); //this regex was formerly used to add colons. please delete commented code. you can add description to the method if you think it requires explanation. in addition, the name implies of converting an Integer to String, while the provided value has type 'long'. perhaps it should be named 'numericMacAddressToString(long macAddress) ? Line 54: Line 55: char[] chars = value.toCharArray(); Line 56: String result = "" + chars[0] + chars[1]; Line 57: for (int pos = 2; pos < value.length(); pos += 2) { Line 52: String value = String.format("%012x", macAddress); Line 53: // String original = StringUtils.join(value.split("(?<=\\G..)"), ':'); //this regex was formerly used to add colons. Line 54: Line 55: char[] chars = value.toCharArray(); Line 56: String result = "" + chars[0] + chars[1]; wouldn't StringBuilder be more efficient in this context instead the string instantiations ? Line 57: for (int pos = 2; pos < value.length(); pos += 2) { Line 58: result += ":" + chars[pos] + chars[pos + 1]; Line 59: } Line 60: return result; -- To view, visit http://gerrit.ovirt.org/25940 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I09f87ebd1ea17d09a974ce6ca3e26bd2454a72fc Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
