SuperMartijn642's Core Lib

Is your feature request related to a problem? Please describe.

Currently model overwrites add about 11 seconds to launch time (measured via profiling). Based on a code analysis, I suspect the problem is in the way you handle ModelBakeEvent.

SuperMartijn642sCoreLib/src/main/java/com/supermartijn642/core/registry/ClientRegistrationHandler.java

Lines 589 to 608 in aa78af8

    
               // Model overwrites 
        
               for(Pair<Predicate<ResourceLocation>,Function<IBakedModel,IBakedModel>> pair : this.modelOverwrites){ 
        
                   // Find all the identifiers which should be replaced 
        
                   List<ResourceLocation> modelIdentifiers = e.getModelRegistry().keySet().stream() 
        
                       .filter(identifier -> pair.left().test(identifier)) 
        
                       .collect(Collectors.toList()); 
        
                   for(ResourceLocation identifier : modelIdentifiers){ 
        
                       if(!e.getModelRegistry().containsKey(identifier)) 
        
                           throw new RuntimeException("No model registered for model overwrite '" + identifier + "'!"); 
        
                       IBakedModel model = e.getModelRegistry().get(identifier); 
        
                       model = pair.right().apply(model); 
        
                       if(model == null) 
        
                           throw new RuntimeException("Model overwrite for '" + identifier + "' returned a null model!"); 
        
                       e.getModelRegistry().put(identifier, model); 
        
                   } 
        
               } 
        
           }

This code has relatively poor time complexity because it requires iterating over every model in the registry and then running every model overwrite's predicate to see if it matches.

Describe the solution you'd like

A potentially better solution would be to implement the modelOverwrites list as a hash map with ResourceLocations or ModelResourceLocations as keys. If necessary multiple entries can be added to this map within the various registration methods to handle replacing multiple models.

The model bake event handler then only needs to iterate over the model registry exactly once, check if that location is in the HashMap, and if so obtain its replacement. This should avoid spending large quantities of time testing each model.

This probably affects 1.16, 1.18, and 1.19, but my profiling has been done on 1.18 using the Enigmatica 8 modpack.

Whoops that code is left over from when I couldn't get the overwrites to work. I changed it to just a predicate so I could more easily test it. However, I probably just moved on to fixing some other issues once it was working and then forgot about.
But damn 11 seconds really is a lot, especially for what is likely only a couple blocks.

I might have some time later today and if not, definitely tomorrow, so shouldn't take too long to get fixed.

I fixed it now in version 1.1.5.

Thank you for notifying me of the issue and for the excellent report 🙂

	// Model overwrites
	for(Pair<Predicate<ResourceLocation>,Function<IBakedModel,IBakedModel>> pair : this.modelOverwrites){
	// Find all the identifiers which should be replaced
	List<ResourceLocation> modelIdentifiers = e.getModelRegistry().keySet().stream()
	.filter(identifier -> pair.left().test(identifier))
	.collect(Collectors.toList());

	for(ResourceLocation identifier : modelIdentifiers){
	if(!e.getModelRegistry().containsKey(identifier))
	throw new RuntimeException("No model registered for model overwrite '" + identifier + "'!");

	IBakedModel model = e.getModelRegistry().get(identifier);
	model = pair.right().apply(model);
	if(model == null)
	throw new RuntimeException("Model overwrite for '" + identifier + "' returned a null model!");

	e.getModelRegistry().put(identifier, model);
	}
	}
	}

Share to