)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0de6ae34a4edff991d7a36a0fbb0593f47a244ab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6a1f9266_71dac102","updated":"2025-10-04 14:52:54.000000000","message":"This needs cleanup and probably some discussion if this speedup worth the code complexity it generates.","commit_id":"9bf97bddd4d9ee9462a19636fd0455e067234036"}],"placement/objects/allocation_candidate.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e4dac2bed3563da775a3b8946a15bef4dbac4963","unresolved":true,"context_lines":[{"line_number":743,"context_line":"        arrs_by_rp_rc \u003d {}"},{"line_number":744,"context_line":"        mappings \u003d collections.defaultdict(set)"},{"line_number":745,"context_line":""},{"line_number":746,"context_line":"        for i, areq in enumerate(candidate):"},{"line_number":747,"context_line":"            partial_candidate.append(areq)"},{"line_number":748,"context_line":""},{"line_number":749,"context_line":"            # build both represenatation of the candidate"},{"line_number":750,"context_line":"            # partial_candidate is just a list of pieces"},{"line_number":751,"context_line":"            # partial_areq is a consoludated view of those pieces"},{"line_number":752,"context_line":""},{"line_number":753,"context_line":"            for arr in areq.resource_requests:"},{"line_number":754,"context_line":"                key \u003d (arr.resource_provider.id, arr.resource_class)"},{"line_number":755,"context_line":"                if key not in arrs_by_rp_rc:"},{"line_number":756,"context_line":"                    arrs_by_rp_rc[key] \u003d rw_ctx.copy_arr_if_needed(arr)"},{"line_number":757,"context_line":"                else:"},{"line_number":758,"context_line":"                    arrs_by_rp_rc[key].amount +\u003d arr.amount"},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"            for suffix, providers in areq.mappings.items():"},{"line_number":761,"context_line":"                mappings[suffix].update(providers)"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"            partial_areq \u003d AllocationRequest("},{"line_number":764,"context_line":"                resource_requests\u003dlist(arrs_by_rp_rc.values()),"},{"line_number":765,"context_line":"                anchor_root_provider_uuid\u003dareq.anchor_root_provider_uuid,"},{"line_number":766,"context_line":"                mappings\u003dmappings)"},{"line_number":767,"context_line":""},{"line_number":768,"context_line":"            if rw_ctx.exceeds_capacity(partial_areq):"},{"line_number":769,"context_line":"                failed_index \u003d i"},{"line_number":770,"context_line":"                break"},{"line_number":771,"context_line":""},{"line_number":772,"context_line":"        if failed_index is None:"},{"line_number":773,"context_line":"            # as we finished the iteration on the candidate building"}],"source_content_type":"text/x-python","patch_set":1,"id":"34341094_79ddb18e","line":770,"range":{"start_line":746,"start_character":0,"end_line":770,"end_character":21},"updated":"2025-10-06 12:43:32.000000000","message":"we chatted about this a liitle on irc.\n\nthis is not a pure inline but rather a interleaving of the capsity check with the pruning\n\nif we want to keep this logic ealily testable we could put it in a helper function\n\nwe would need to pass in arrs_by_rp_rc, mappings and partial_candidate so that the function can mutate them and return failed_index as either None or an index if the capscity check failed\n\n\n\n```suggestion\n    def _partal_prune(arrs_by_rp_rc,mappings,partial_candidate):\n        for i, areq in enumerate(candidate):\n            partial_candidate.append(areq)\n\n            # build both represenatation of the candidate\n            # partial_candidate is just a list of pieces\n            # partial_areq is a consoludated view of those pieces\n\n            for arr in areq.resource_requests:\n                key \u003d (arr.resource_provider.id, arr.resource_class)\n                if key not in arrs_by_rp_rc:\n                    arrs_by_rp_rc[key] \u003d rw_ctx.copy_arr_if_needed(arr)\n                else:\n                    arrs_by_rp_rc[key].amount +\u003d arr.amount\n\n            for suffix, providers in areq.mappings.items():\n                mappings[suffix].update(providers)\n\n            partial_areq \u003d AllocationRequest(\n                resource_requests\u003dlist(arrs_by_rp_rc.values()),\n                anchor_root_provider_uuid\u003dareq.anchor_root_provider_uuid,\n                mappings\u003dmappings)\n\n            if rw_ctx.exceeds_capacity(partial_areq):\n                return i\n     failed_index\u003d self._partal_prune(arrs_by_rp_rc,mappings,partial_candidate)\n```\n\nthat shoudl allow you to keep the partial pruning optimization without neeing to fuly inline the function but im not sure if there is enouch of a unit testing benifit to that vs just testing all of product. i think yes but i do not have a stong opion on this either way.","commit_id":"9bf97bddd4d9ee9462a19636fd0455e067234036"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"919fc303d3bce7104038509195171943326b8f8b","unresolved":true,"context_lines":[{"line_number":743,"context_line":"        arrs_by_rp_rc \u003d {}"},{"line_number":744,"context_line":"        mappings \u003d collections.defaultdict(set)"},{"line_number":745,"context_line":""},{"line_number":746,"context_line":"        for i, areq in enumerate(candidate):"},{"line_number":747,"context_line":"            partial_candidate.append(areq)"},{"line_number":748,"context_line":""},{"line_number":749,"context_line":"            # build both represenatation of the candidate"},{"line_number":750,"context_line":"            # partial_candidate is just a list of pieces"},{"line_number":751,"context_line":"            # partial_areq is a consoludated view of those pieces"},{"line_number":752,"context_line":""},{"line_number":753,"context_line":"            for arr in areq.resource_requests:"},{"line_number":754,"context_line":"                key \u003d (arr.resource_provider.id, arr.resource_class)"},{"line_number":755,"context_line":"                if key not in arrs_by_rp_rc:"},{"line_number":756,"context_line":"                    arrs_by_rp_rc[key] \u003d rw_ctx.copy_arr_if_needed(arr)"},{"line_number":757,"context_line":"                else:"},{"line_number":758,"context_line":"                    arrs_by_rp_rc[key].amount +\u003d arr.amount"},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"            for suffix, providers in areq.mappings.items():"},{"line_number":761,"context_line":"                mappings[suffix].update(providers)"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"            partial_areq \u003d AllocationRequest("},{"line_number":764,"context_line":"                resource_requests\u003dlist(arrs_by_rp_rc.values()),"},{"line_number":765,"context_line":"                anchor_root_provider_uuid\u003dareq.anchor_root_provider_uuid,"},{"line_number":766,"context_line":"                mappings\u003dmappings)"},{"line_number":767,"context_line":""},{"line_number":768,"context_line":"            if rw_ctx.exceeds_capacity(partial_areq):"},{"line_number":769,"context_line":"                failed_index \u003d i"},{"line_number":770,"context_line":"                break"},{"line_number":771,"context_line":""},{"line_number":772,"context_line":"        if failed_index is None:"},{"line_number":773,"context_line":"            # as we finished the iteration on the candidate building"}],"source_content_type":"text/x-python","patch_set":1,"id":"3e14fe26_9cb4f82a","line":770,"range":{"start_line":746,"start_character":0,"end_line":770,"end_character":21},"in_reply_to":"34341094_79ddb18e","updated":"2025-10-07 12:32:53.000000000","message":"I added unit test regardless of this comment. \n\nPulling this step out and passing all the local variables it modifies to it doe not really help me discussion what happens here. So for now I did not implemented this function.","commit_id":"9bf97bddd4d9ee9462a19636fd0455e067234036"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"68b4d9aee4162b2e9c8927f0ab53b71dfb87af61","unresolved":true,"context_lines":[{"line_number":743,"context_line":"        arrs_by_rp_rc \u003d {}"},{"line_number":744,"context_line":"        mappings \u003d collections.defaultdict(set)"},{"line_number":745,"context_line":""},{"line_number":746,"context_line":"        for i, areq in enumerate(candidate):"},{"line_number":747,"context_line":"            partial_candidate.append(areq)"},{"line_number":748,"context_line":""},{"line_number":749,"context_line":"            # build both represenatation of the candidate"},{"line_number":750,"context_line":"            # partial_candidate is just a list of pieces"},{"line_number":751,"context_line":"            # partial_areq is a consoludated view of those pieces"},{"line_number":752,"context_line":""},{"line_number":753,"context_line":"            for arr in areq.resource_requests:"},{"line_number":754,"context_line":"                key \u003d (arr.resource_provider.id, arr.resource_class)"},{"line_number":755,"context_line":"                if key not in arrs_by_rp_rc:"},{"line_number":756,"context_line":"                    arrs_by_rp_rc[key] \u003d rw_ctx.copy_arr_if_needed(arr)"},{"line_number":757,"context_line":"                else:"},{"line_number":758,"context_line":"                    arrs_by_rp_rc[key].amount +\u003d arr.amount"},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"            for suffix, providers in areq.mappings.items():"},{"line_number":761,"context_line":"                mappings[suffix].update(providers)"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"            partial_areq \u003d AllocationRequest("},{"line_number":764,"context_line":"                resource_requests\u003dlist(arrs_by_rp_rc.values()),"},{"line_number":765,"context_line":"                anchor_root_provider_uuid\u003dareq.anchor_root_provider_uuid,"},{"line_number":766,"context_line":"                mappings\u003dmappings)"},{"line_number":767,"context_line":""},{"line_number":768,"context_line":"            if rw_ctx.exceeds_capacity(partial_areq):"},{"line_number":769,"context_line":"                failed_index \u003d i"},{"line_number":770,"context_line":"                break"},{"line_number":771,"context_line":""},{"line_number":772,"context_line":"        if failed_index is None:"},{"line_number":773,"context_line":"            # as we finished the iteration on the candidate building"}],"source_content_type":"text/x-python","patch_set":1,"id":"aabde019_530a322e","line":770,"range":{"start_line":746,"start_character":0,"end_line":770,"end_character":21},"in_reply_to":"3e14fe26_9cb4f82a","updated":"2025-10-08 15:26:59.000000000","message":"After discussing this patch with Sean and Dan on a call I was convinced to do the refactoring asked here.","commit_id":"9bf97bddd4d9ee9462a19636fd0455e067234036"}],"placement/tests/functional/test_allocation_candidates.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0de6ae34a4edff991d7a36a0fbb0593f47a244ab","unresolved":true,"context_lines":[{"line_number":227,"context_line":"        # algorithm from exponential to factorial."},{"line_number":228,"context_line":"        #"},{"line_number":229,"context_line":"        # This runs in 1.3 seconds. If you bump this from 1000 to 10k maximum"},{"line_number":230,"context_line":"        # candidates then it will run in 103 seconds."},{"line_number":231,"context_line":"        self.conf_fixture.conf.set_override("},{"line_number":232,"context_line":"            \"optimize_for_wide_provider_trees\", True, group\u003d\"workarounds\")"},{"line_number":233,"context_line":"        self.conf_fixture.conf.set_override("}],"source_content_type":"text/x-python","patch_set":1,"id":"c3c3c615_590d1a81","line":230,"updated":"2025-10-04 14:52:54.000000000","message":"This feels like an outlier, even 21_21 was speed up a lot by this change. I guess there is another hot spot in this case as with 10k max_allocation_candidates we are closing in to to the number of max valid candidates which is 40k. In 21_21 we are far from that max.\n\nmaybe the new hostpost is exceeds_capacity. Probably I can inline that as well and make it cumulative...","commit_id":"9bf97bddd4d9ee9462a19636fd0455e067234036"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c203c6ba1890b9b0ccc56bf6e63a796e62824cca","unresolved":true,"context_lines":[{"line_number":227,"context_line":"        # algorithm from exponential to factorial."},{"line_number":228,"context_line":"        #"},{"line_number":229,"context_line":"        # This runs in 1.3 seconds. If you bump this from 1000 to 10k maximum"},{"line_number":230,"context_line":"        # candidates then it will run in 103 seconds."},{"line_number":231,"context_line":"        self.conf_fixture.conf.set_override("},{"line_number":232,"context_line":"            \"optimize_for_wide_provider_trees\", True, group\u003d\"workarounds\")"},{"line_number":233,"context_line":"        self.conf_fixture.conf.set_override("}],"source_content_type":"text/x-python","patch_set":1,"id":"939a90f8_50001d9b","line":230,"in_reply_to":"c3c3c615_590d1a81","updated":"2025-10-06 07:25:36.000000000","message":"As the next patch in the series shows the inlining exceeds_capacity does not help with this case either so probably we need to live with it. My next best guest would be to figure out if we can shave the cost of hashing and eqaulity checks of AllocationRequest and the profiler output highlight that area. However at a first glance I don\u0027t see an easy way to do it. Making that class immutable probably help but then we would need to copy of them more that would probably eat the gain.","commit_id":"9bf97bddd4d9ee9462a19636fd0455e067234036"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0de6ae34a4edff991d7a36a0fbb0593f47a244ab","unresolved":true,"context_lines":[{"line_number":262,"context_line":"            expected_candidates\u003d1000, expected_computes_with_candidates\u003d1)"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"    def test_many_non_viable_candidates_21_21(self):"},{"line_number":265,"context_line":"        # This is runs in 4 seconds"},{"line_number":266,"context_line":"        self.conf_fixture.conf.set_override("},{"line_number":267,"context_line":"            \"optimize_for_wide_provider_trees\", True, group\u003d\"workarounds\")"},{"line_number":268,"context_line":"        self.conf_fixture.conf.set_override("}],"source_content_type":"text/x-python","patch_set":1,"id":"bb9fd63d_3667b12f","line":265,"updated":"2025-10-04 14:52:54.000000000","message":"I would say this worth the extra complexity of the inlining but we can disuss","commit_id":"9bf97bddd4d9ee9462a19636fd0455e067234036"}],"placement/tests/unit/objects/test_allocation_candidate.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"68208d6913dfbc63acc822277f43c92a99d9b482","unresolved":false,"context_lines":[{"line_number":224,"context_line":"            },"},{"line_number":225,"context_line":"        }"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"        orig_consolidate \u003d ac_obj._consolidate_allocation_requests"},{"line_number":228,"context_line":"        consolidate_calls \u003d []"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"        def wrap_consolidate_allocation_requests(areq_list, rw_ctx):"}],"source_content_type":"text/x-python","patch_set":7,"id":"0a6d5cc6_55f5d8a7","side":"PARENT","line":227,"updated":"2025-10-07 12:30:05.000000000","message":"we inlined this call to use the cumulative nature of it to avoid some repeated calculation. So it is hard to show now that we do less intermediate consolidation. But we do as it is show by the speed up in the functional test.","commit_id":"e3cebf6f2263513149bfc3e68e6d638ea7e7e54f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"68208d6913dfbc63acc822277f43c92a99d9b482","unresolved":false,"context_lines":[{"line_number":336,"context_line":"            ({\"G1\": {\"RP1\"}, \"G2\": {\"RP3\"}, \"G3\": {\"RP1\"}}, True),"},{"line_number":337,"context_line":"            ({\"G1\": {\"RP1\"}}, False),"},{"line_number":338,"context_line":"            ({\"G1\": {\"RP1\"}, \"G2\": {\"RP3\"}}, False),"},{"line_number":339,"context_line":"            ({\"G1\": {\"RP1\"}, \"G2\": {\"RP3\"}, \"G3\": {\"RP2\"}}, False),"},{"line_number":340,"context_line":"            # ..."},{"line_number":341,"context_line":"        ]"},{"line_number":342,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"805a1e6b_7e767d2a","side":"PARENT","line":339,"updated":"2025-10-07 12:30:05.000000000","message":"the number of tests did not change. I optimized some of that in the next patch in the series but that did not helped much. Optimizing or caching the pre-fix tests would require more work. See explanation in https://review.opendev.org/c/openstack/placement/+/962776/11#message-ff870915d38b39f98d801a8697b9abc3fe670be1","commit_id":"e3cebf6f2263513149bfc3e68e6d638ea7e7e54f"}]}