Opened 3 years ago

Closed 2 years ago

#2239 closed defect (done)

The VariableStoreView unnecessarily tries to serialize objects.

Reported by: bburlacu Owned by: jkarder
Priority: medium Milestone: HeuristicLab 3.3.11
Component: Scripting.Views Version: 3.3.10
Keywords: Cc:

Description

The IsSerializable() method is expensive for large objects, as it needs to serialize the object. Additionally, this method is called every time a variable name is changed (which is not really necessary). A simple fix would be to use a Dictionary<Type, bool> to store this information when a variable is added to avoid calling the serializer when the type is known. I attached a patch to illustrate this quick fix.

Attachments (1)

VariableStoreView.cs.patch (2.0 KB) - added by bburlacu 3 years ago.

Download all attachments as: .zip

Change History (17)

Changed 3 years ago by bburlacu

comment:1 Changed 3 years ago by jkarder

  • Status changed from new to accepted

comment:2 Changed 3 years ago by jkarder

r11437: applied a slightly modified version of bburlacu's patch

comment:3 Changed 3 years ago by jkarder

Thanks for the hint. Your code works but does not consider that object graphs can change and that after such changes occur, objects might not be serializable anymore. As this is currently only possible within CSharpScripts themselves, this issue will be ignored for now. Thanks again!

comment:4 Changed 3 years ago by jkarder

  • Owner changed from jkarder to mkommend
  • Status changed from accepted to reviewing

comment:5 follow-up: Changed 3 years ago by abeham

I am also using the variable store in the programmable problem. Does this affect the programmable problem also?

comment:6 in reply to: ↑ 5 Changed 3 years ago by jkarder

Replying to abeham:

I am also using the variable store in the programmable problem. Does this affect the programmable problem also?

I will look into it.

comment:7 Changed 3 years ago by jkarder

  • Owner changed from mkommend to jkarder
  • Status changed from reviewing to assigned

comment:8 Changed 3 years ago by jkarder

  • Status changed from assigned to accepted

comment:9 Changed 3 years ago by jkarder

r11439: fixed IsSerializable

comment:10 Changed 3 years ago by jkarder

r11472: actually fixed IsSerializable

comment:11 Changed 3 years ago by jkarder

  • Owner changed from jkarder to mkommend
  • Status changed from accepted to reviewing

comment:12 Changed 2 years ago by mkommend

  • Status changed from reviewing to readytorelease

Reviewed r11437, r11439, and r11472.

comment:13 Changed 2 years ago by mkommend

Must be released in combination with #2262.

comment:14 Changed 2 years ago by mkommend

  • Status changed from readytorelease to reviewing

comment:15 Changed 2 years ago by mkommend

  • Owner changed from mkommend to jkarder
  • Status changed from reviewing to readytorelease

comment:16 Changed 2 years ago by jkarder

  • Resolution set to done
  • Status changed from readytorelease to closed

r11907: merged r11437, r11439 and r11472 into stable

Note: See TracTickets for help on using tickets.