pombreda / google-guice

Automatically exported from code.google.com/p/google-guice
Apache License 2.0
0 stars 1 forks source link

Avoid unnecessary object creation if .withSource doesn't change the source #776

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Currently RecordingBinder.withSource always creates a new RecordingBinder. This 
can lead to a lot of unnecessary object creation when replaying/applying 
bindings because each binding will call binder.withSource(getSource())...

Original issue reported on code.google.com by mccu...@gmail.com on 22 Oct 2013 at 4:04

GoogleCodeExporter commented 9 years ago
Suggested patch; avoids creating new RecordingBinder (ditto for Errors) when 
the source passed into withSource is exactly the same as the current one.

Original comment by mccu...@gmail.com on 22 Oct 2013 at 4:17

Attachments:

GoogleCodeExporter commented 9 years ago
Regarding your patch, are there so many cases that the same source is passed to 
withSource()?

Original comment by salman...@google.com on 22 Oct 2013 at 4:29

GoogleCodeExporter commented 9 years ago
As mentioned above, when replaying/applying bindings using the SPI (for example 
when using Modules.override) each binding will call 
binder.withSource(getSource()) and therefore a new RecordingBinder will be 
created for each binding even though they will often all have the same source.

Original comment by mccu...@gmail.com on 22 Oct 2013 at 4:37

GoogleCodeExporter commented 9 years ago
Right, I agree that it can improve the current situation.
I have looked at the problem a few months ago, however at the time I was more 
interested in a solution that avoids a new binder creation in all cases.

Original comment by salman...@google.com on 22 Oct 2013 at 4:52

GoogleCodeExporter commented 9 years ago
If it's a quick/easy win for the SPI case, we might as well submit this patch 
and then look for more optimizations later.

Original comment by sberlin on 22 Oct 2013 at 6:12

GoogleCodeExporter commented 9 years ago

Original comment by sberlin on 6 Dec 2013 at 10:52