api-issue-tracker icon indicating copy to clipboard operation
api-issue-tracker copied to clipboard

The trim is very time consuming in large model

Open LItterBoy-GB opened this issue 4 years ago • 7 comments

Bug Reports

Please include the following:

  1. SketchUp/LayOut Version: Sketchup 2019 pro
  2. OS Platform: windows 10 x64

g1 trim g2

small model : 0.022106 large model : 0.202183

# small
g1 = Sketchup.active_model.entities.find { |ent|
  ent.is_a?(Sketchup::Group) && ent&.material&.name == 'Marc_Wrist'
}

g2 = Sketchup.active_model.entities.find { |ent|
  ent.is_a?(Sketchup::Group) && ent&.material&.name == 'Marc_Shoes3'
}

g3 = Sketchup.active_model.entities.find { |ent|
  ent.is_a?(Sketchup::Group) && ent&.material&.name == 'Marc_Hair'
}
s = Time.now
g1.trim g2
puts "small : #{Time.now - s}"  # small : 0.022106
Sketchup.undo

# large
g1 = Sketchup.active_model.entities.find { |ent|
  ent.is_a?(Sketchup::Group) && ent&.material&.name == 'Marc_Wrist'
}

g2 = Sketchup.active_model.entities.find { |ent|
  ent.is_a?(Sketchup::Group) && ent&.material&.name == 'Marc_Shoes3'
}

g3 = Sketchup.active_model.entities.find { |ent|
  ent.is_a?(Sketchup::Group) && ent&.material&.name == 'Marc_Hair'
}
Sketchup.active_model.start_operation 'copy_make_g3',true
1000.times do |i|
  tr = Geom::Transformation.new(Geom::Point3d.new(0,10*(i+1),0))
  g4 = g3.copy
  g4 = g4.make_unique
  g4.transform!(tr)
end
Sketchup.active_model.commit_operation

s = Time.now
g1.trim g2
puts "large : #{Time.now - s}"  # small : 0.202183

Sketchup.undo

image

LItterBoy-GB avatar Aug 30 '21 06:08 LItterBoy-GB

What if g1 and g2 are first moved into a temporary group before the trim ? Then the temp group exploded afterward.

DanRathbun avatar Aug 31 '21 10:08 DanRathbun

https://forums.sketchup.com/t/the-trim-is-very-time-consuming-in-large-model/171986/11?u=tt_su

It looks to relate to the groups being trimmed being made unique, whereupon SU it looking for a new unique name. When you have a lot of other groups in the model a lot of the definitions start with “Group” and it has to traverse for longer to find a unique name. Logged as an performance issue.

thomthom avatar Sep 01 '21 12:09 thomthom

Logged as: SKEXT-3173

sketchupbot avatar Sep 06 '21 06:09 sketchupbot

@DanRathbun : What if g1 and g2 are first moved into a temporary group before the trim ? Then the temp group exploded afterward.

@LItterBoy-GB said in public forum post ...

By the way, the function ( add_group ) has the same problem.

@thomthom replied ...

Yea, it also tries to find a unique name.

@LItterBoy-GB also said in public forum post ...

This means that I can avoid the problem by changing the name (no start with “Group”).

@thomthom replied ...

That might work… worth a try.

One problem with naming and a temporary group is that none of the boolean methods nor the Entities#add_group method accept a name string argument for the result.

But the main problem is that the Group#trim method names the definition and instance results "Difference" each time a trim is done (regardless of the starting names) and does not uniquify the instance name, which can result in multiple instances named "Difference" (but they are instances of different definitions.) The first resultant definition gets the name "Difference", and thereafter they are uniquified with #n suffixes.

So the problem prefix is not necessarily "Group" (at least when it comes to the #trim method.)

And no matter what names is used for the result, the entire definition list of names would still be compared, which is where the time is lost.


New arguments cannot be added to these existing methods (boolean and #add_group,) to control the result names ... but I wonder if:

The internals could perhaps just use the Time.now.to_i or Time.now.to_f as a suffix. (Ie, timestamp the results.) The internals would keep a temporary array of definition timestamps during an operation and check it for unique numbers (which should be faster than comparing character strings.) Or, (during the operation) remember the last timestamp used and just increment from it for a new definition suffix.

The result names could combine both the operation name and the timestamp to generate known unique names.

DanRathbun avatar Sep 06 '21 22:09 DanRathbun

Hi LItterBoy-GB,

While reviewing old issues, I encountered this one. I am seeing huge improvement in the output values :

  1. SketchUp version : SU2023.1 small : 0.0039327 large : 0.0043554

  2. SketchUp version : SU2024.0 small : 0.0060735 large : 0.0043472

  3. SketchUp version : SU2025.0 small : 0.005823 large : 0.0061193

Could you please confirm if you're still experiencing this issue?

Thanks

kalpana-ghodeswar avatar May 15 '25 10:05 kalpana-ghodeswar

Could you please confirm if you're still experiencing this issue?

Yes.The problem still exists. The small model has only two groups. The large model has 16054 components and 38600 groups

Image

LItterBoy-GB avatar May 20 '25 03:05 LItterBoy-GB

Thanks for the feedback LItterBoy-GB. I will add this info to the Jira ticket. Hopefully we will be able to look into this issue soon.

kalpana-ghodeswar avatar May 20 '25 09:05 kalpana-ghodeswar