RFC60 finalization
|
|
UV-2
|
Hi all,
I looked into the RFC60 code again and added some more features to it e.g. finding label colors. However, a redirection of referenced featureIds which would cause a FDO lookup seems prohibitive expensive in this context. I would not like to add this into the code simply as it could increase delay tile computation a lot. So the interesting test here are performance related. Simple unit-tests are not sufficient for this. Creating complex test data to test performance issues is tedious and very badly motivated!!! (specification vs. implementation from same hand) Secondly, it think it is wrong to request the implementation of the new stylization for such transparent feature addition. RFC60 works fine for our case, why should we implement something that does not harm anything but what we do not need? This is not thought along open source paradigms. Open source is more like: Who needs it should code it! So we should leave this open and the next person that has real test data which doesn't work can post a defect! Very easy process! RFC60 is not trivial at all but it solves a real problem in real maps. To block the inclusion of RFC60 into the code base on those matters is simply wrong in open source terms as it keeps others from using a new working feature. _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
Walt Welton-Lair
|
> Secondly, it think it is wrong to request the implementation of the new
> stylization for such transparent feature addition. > RFC60 works fine for our case, why should we implement something that > does not harm anything but what we do not need? The way your code is designed - where it does the work of grabbing the colors inside the MdfModel project - WILL NOT WORK for enhanced stylization in which there are referenced symbol definitions. It's not possible to load referenced symbols from the resource service from inside the MdfModel code. This is not a minor defect. It requires that a lot of your code be refactored. I already explained the problem last March. Here's the excerpt: "With the new enhanced stylization (RFC 14) the layer definition can reference symbol definitions (in addition to inlining them). The symbols, of course, define colors which need to be accounted for. Accessing referenced symbol definitions requires the resource service - we need to get the symbol definition resource from the service. The MdfModel project (where VectorLayerDefinition is stored) does not have access (nor want access) to the resource service. So if you want to properly support the new enhanced stylization with your RFC (you should) then we'll probably have to move this ComputeUsedColors method somewhere else. There's a method - MgMappingUtil::ComputeStylizationOffset - which does something analogous to ComputeUsedColors, so possibly we can add ComputeUsedColors to MgMappingUtil." So even if you don't add code to actually parse colors for enhanced style layers, you need to keep the bigger picture in mind and design your new code so it will be easy to add this support. All along that's the real reason why I wanted you to take a look at this. So that you would come to realize that your current design doesn't fully work as is. > Open source is more like: Who needs it should code it! True, but it's not an excuse for not doing the right thing. People can't just add anything they want without taking account the bigger picture. That's my take on this. Walt -----Original Message----- From: [hidden email] [mailto:[hidden email]] On Behalf Of UV Sent: Friday, October 23, 2009 9:03 AM To: MapGuide Internals Mail List Subject: [mapguide-internals] RFC60 finalization Hi all, I looked into the RFC60 code again and added some more features to it e.g. finding label colors. However, a redirection of referenced featureIds which would cause a FDO lookup seems prohibitive expensive in this context. I would not like to add this into the code simply as it could increase delay tile computation a lot. So the interesting test here are performance related. Simple unit-tests are not sufficient for this. Creating complex test data to test performance issues is tedious and very badly motivated!!! (specification vs. implementation from same hand) Secondly, it think it is wrong to request the implementation of the new stylization for such transparent feature addition. RFC60 works fine for our case, why should we implement something that does not harm anything but what we do not need? This is not thought along open source paradigms. Open source is more like: Who needs it should code it! So we should leave this open and the next person that has real test data which doesn't work can post a defect! Very easy process! RFC60 is not trivial at all but it solves a real problem in real maps. To block the inclusion of RFC60 into the code base on those matters is simply wrong in open source terms as it keeps others from using a new working feature. _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
Jason Birch
|
It seems to me that we have frequently (too often perhaps?) favoured
expediency over correctness when faced with resourcing or timeline issues. In this case, I'm in favour of placing the burden of change on the whoever deems that it is critical to support pallette reservation for attribute-based stylization. Jason On 2009-10-23, Walt Welton-Lair <[hidden email]> wrote: >> Secondly, it think it is wrong to request the implementation of the new >> stylization for such transparent feature addition. >> RFC60 works fine for our case, why should we implement something that >> does not harm anything but what we do not need? > > The way your code is designed - where it does the work of grabbing the > colors inside the MdfModel project - WILL NOT WORK for enhanced stylization > in which there are referenced symbol definitions. It's not possible to load > referenced symbols from the resource service from inside the MdfModel code. > This is not a minor defect. It requires that a lot of your code be > refactored. I already explained the problem last March. Here's the > excerpt: > > "With the new enhanced stylization (RFC 14) the layer definition can > reference symbol definitions (in addition to inlining them). The symbols, > of course, define colors which need to be accounted for. Accessing > referenced symbol definitions requires the resource service - we need to get > the symbol definition resource from the service. The MdfModel project > (where VectorLayerDefinition is stored) does not have access (nor want > access) to the resource service. So if you want to properly support the new > enhanced stylization with your RFC (you should) then we'll probably have to > move this ComputeUsedColors method somewhere else. There's a method - > MgMappingUtil::ComputeStylizationOffset - which does something analogous to > ComputeUsedColors, so possibly we can add ComputeUsedColors to > MgMappingUtil." > > So even if you don't add code to actually parse colors for enhanced style > layers, you need to keep the bigger picture in mind and design your new code > so it will be easy to add this support. > > All along that's the real reason why I wanted you to take a look at this. > So that you would come to realize that your current design doesn't fully > work as is. > > >> Open source is more like: Who needs it should code it! > > True, but it's not an excuse for not doing the right thing. People can't > just add anything they want without taking account the bigger picture. > That's my take on this. > > > Walt > > -----Original Message----- > From: [hidden email] > [mailto:[hidden email]] On Behalf Of UV > Sent: Friday, October 23, 2009 9:03 AM > To: MapGuide Internals Mail List > Subject: [mapguide-internals] RFC60 finalization > > Hi all, > > I looked into the RFC60 code again and added some more features to it > e.g. finding label colors. > > However, a redirection of referenced featureIds which would cause a FDO > lookup seems prohibitive expensive in this context. > I would not like to add this into the code simply as it could increase > delay tile computation a lot. > So the interesting test here are performance related. Simple unit-tests > are not sufficient for this. > Creating complex test data to test performance issues is tedious and > very badly motivated!!! (specification vs. implementation from same hand) > > Secondly, it think it is wrong to request the implementation of the new > stylization for such transparent feature addition. > RFC60 works fine for our case, why should we implement something that > does not harm anything but what we do not need? > This is not thought along open source paradigms. Open source is more > like: Who needs it should code it! > So we should leave this open and the next person that has real test data > which doesn't work can post a defect! > Very easy process! > > > RFC60 is not trivial at all but it solves a real problem in real maps. > To block the inclusion of RFC60 into the code base on those matters is > simply wrong in open source terms as it keeps others from using a new > working feature. > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
Tom Fukushima
|
I'm trying to understand this...so correct me if I'm wrong. But it seems that the problem is not with attribute-based stylization. It's that if someone uses referenced symbols (that is, the LayerDefinition specifies a resource using a resource ID such as Library://mysymbols/a.SymbolDefinition instead of embedded XML for a SymbolDefinition in the LayerDefinition) then a large part of the code will need to be rewritten to support it.
If the above is the case, I understand that we need to make tradeoffs all the time, and this may be where we need to make one. I think though that it would be advantageous for the current developer to do the requested refactoring now while it is fresh. Later, if defects are found or refinements (e.g., attribute-based stylization) are done, they could be addressed at that time---and this would be manageable because wouldn't require refactoring large parts of the code. Could we get some idea of the work that would be required to get support for the referenced symbol definitions? Note that I'm not saying it has to be done (but of course I would prefer it to be done), I'm trying to get more information on this sticking point (I believe it is the only one right?) to try to make a judgment. Tom -----Original Message----- From: [hidden email] [mailto:[hidden email]] On Behalf Of Jason Birch Sent: Friday, October 23, 2009 9:24 AM To: MapGuide Internals Mail List Subject: Re: [mapguide-internals] RFC60 finalization It seems to me that we have frequently (too often perhaps?) favoured expediency over correctness when faced with resourcing or timeline issues. In this case, I'm in favour of placing the burden of change on the whoever deems that it is critical to support pallette reservation for attribute-based stylization. Jason On 2009-10-23, Walt Welton-Lair <[hidden email]> wrote: >> Secondly, it think it is wrong to request the implementation of the new >> stylization for such transparent feature addition. >> RFC60 works fine for our case, why should we implement something that >> does not harm anything but what we do not need? > > The way your code is designed - where it does the work of grabbing the > colors inside the MdfModel project - WILL NOT WORK for enhanced stylization > in which there are referenced symbol definitions. It's not possible to load > referenced symbols from the resource service from inside the MdfModel code. > This is not a minor defect. It requires that a lot of your code be > refactored. I already explained the problem last March. Here's the > excerpt: > > "With the new enhanced stylization (RFC 14) the layer definition can > reference symbol definitions (in addition to inlining them). The symbols, > of course, define colors which need to be accounted for. Accessing > referenced symbol definitions requires the resource service - we need to get > the symbol definition resource from the service. The MdfModel project > (where VectorLayerDefinition is stored) does not have access (nor want > access) to the resource service. So if you want to properly support the new > enhanced stylization with your RFC (you should) then we'll probably have to > move this ComputeUsedColors method somewhere else. There's a method - > MgMappingUtil::ComputeStylizationOffset - which does something analogous to > ComputeUsedColors, so possibly we can add ComputeUsedColors to > MgMappingUtil." > > So even if you don't add code to actually parse colors for enhanced style > layers, you need to keep the bigger picture in mind and design your new code > so it will be easy to add this support. > > All along that's the real reason why I wanted you to take a look at this. > So that you would come to realize that your current design doesn't fully > work as is. > > >> Open source is more like: Who needs it should code it! > > True, but it's not an excuse for not doing the right thing. People can't > just add anything they want without taking account the bigger picture. > That's my take on this. > > > Walt > > -----Original Message----- > From: [hidden email] > [mailto:[hidden email]] On Behalf Of UV > Sent: Friday, October 23, 2009 9:03 AM > To: MapGuide Internals Mail List > Subject: [mapguide-internals] RFC60 finalization > > Hi all, > > I looked into the RFC60 code again and added some more features to it > e.g. finding label colors. > > However, a redirection of referenced featureIds which would cause a FDO > lookup seems prohibitive expensive in this context. > I would not like to add this into the code simply as it could increase > delay tile computation a lot. > So the interesting test here are performance related. Simple unit-tests > are not sufficient for this. > Creating complex test data to test performance issues is tedious and > very badly motivated!!! (specification vs. implementation from same hand) > > Secondly, it think it is wrong to request the implementation of the new > stylization for such transparent feature addition. > RFC60 works fine for our case, why should we implement something that > does not harm anything but what we do not need? > This is not thought along open source paradigms. Open source is more > like: Who needs it should code it! > So we should leave this open and the next person that has real test data > which doesn't work can post a defect! > Very easy process! > > > RFC60 is not trivial at all but it solves a real problem in real maps. > To block the inclusion of RFC60 into the code base on those matters is > simply wrong in open source terms as it keeps others from using a new > working feature. > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
|
Walt Welton-Lair
|
Yes, my main concern is not with attribute-based stylization. I want the current code to be updated so that it can properly accommodate enhanced stylization going forward.
The refactoring work: * Move changes that were done in MdfModel to collect colors into a method in MgMappingUtil. For now that method will only collect colors for the legacy stylization. To add support for enhanced stylization (inlined symbols): * add code to the above method which iterates over the symbol definition object model and collects colors * not a huge task - roughly 1-2 days work is my estimate. To add support for enhanced stylization (referenced symbols): * Assuming inlined symbol support is present, update the method to make use of the SEMgSymbolManager class (in MappingService) to load referenced symbols. Once loaded, pass the symbol to the code which extracts colors for a symbol definition (added during the previous task). * relatively straightforward task - 1 day is sufficient BTW, even if we completely ignore enhanced stylization the current RFC 60 implementation has the same problem when it comes to point layers styled using a symbol library. The layer definition references a DWF symbol library resource and the name of the symbol to use. The current RFC 60 code will be unable to load the DWF symbol and take into account any colors defined in the DWF symbol. But after the refactoring, this would now be possible. Similar to the enhanced stylization, the color collecting code could be updated to use of the RSMgSymbolManager class to load referenced DWF symbols. And of course there would need to be a method to parse a DWF symbol and collect its colors. For both of these, if we do the refactoring then the color collecting code is ready for future enhancements to support missing cases like the above. Walt -----Original Message----- From: [hidden email] [mailto:[hidden email]] On Behalf Of Tom Fukushima Sent: Friday, October 23, 2009 11:56 AM To: MapGuide Internals Mail List Subject: RE: [mapguide-internals] RFC60 finalization I'm trying to understand this...so correct me if I'm wrong. But it seems that the problem is not with attribute-based stylization. It's that if someone uses referenced symbols (that is, the LayerDefinition specifies a resource using a resource ID such as Library://mysymbols/a.SymbolDefinition instead of embedded XML for a SymbolDefinition in the LayerDefinition) then a large part of the code will need to be rewritten to support it. If the above is the case, I understand that we need to make tradeoffs all the time, and this may be where we need to make one. I think though that it would be advantageous for the current developer to do the requested refactoring now while it is fresh. Later, if defects are found or refinements (e.g., attribute-based stylization) are done, they could be addressed at that time---and this would be manageable because wouldn't require refactoring large parts of the code. Could we get some idea of the work that would be required to get support for the referenced symbol definitions? Note that I'm not saying it has to be done (but of course I would prefer it to be done), I'm trying to get more information on this sticking point (I believe it is the only one right?) to try to make a judgment. Tom -----Original Message----- From: [hidden email] [mailto:[hidden email]] On Behalf Of Jason Birch Sent: Friday, October 23, 2009 9:24 AM To: MapGuide Internals Mail List Subject: Re: [mapguide-internals] RFC60 finalization It seems to me that we have frequently (too often perhaps?) favoured expediency over correctness when faced with resourcing or timeline issues. In this case, I'm in favour of placing the burden of change on the whoever deems that it is critical to support pallette reservation for attribute-based stylization. Jason On 2009-10-23, Walt Welton-Lair <[hidden email]> wrote: >> Secondly, it think it is wrong to request the implementation of the new >> stylization for such transparent feature addition. >> RFC60 works fine for our case, why should we implement something that >> does not harm anything but what we do not need? > > The way your code is designed - where it does the work of grabbing the > colors inside the MdfModel project - WILL NOT WORK for enhanced stylization > in which there are referenced symbol definitions. It's not possible to load > referenced symbols from the resource service from inside the MdfModel code. > This is not a minor defect. It requires that a lot of your code be > refactored. I already explained the problem last March. Here's the > excerpt: > > "With the new enhanced stylization (RFC 14) the layer definition can > reference symbol definitions (in addition to inlining them). The symbols, > of course, define colors which need to be accounted for. Accessing > referenced symbol definitions requires the resource service - we need to get > the symbol definition resource from the service. The MdfModel project > (where VectorLayerDefinition is stored) does not have access (nor want > access) to the resource service. So if you want to properly support the new > enhanced stylization with your RFC (you should) then we'll probably have to > move this ComputeUsedColors method somewhere else. There's a method - > MgMappingUtil::ComputeStylizationOffset - which does something analogous to > ComputeUsedColors, so possibly we can add ComputeUsedColors to > MgMappingUtil." > > So even if you don't add code to actually parse colors for enhanced style > layers, you need to keep the bigger picture in mind and design your new code > so it will be easy to add this support. > > All along that's the real reason why I wanted you to take a look at this. > So that you would come to realize that your current design doesn't fully > work as is. > > >> Open source is more like: Who needs it should code it! > > True, but it's not an excuse for not doing the right thing. People can't > just add anything they want without taking account the bigger picture. > That's my take on this. > > > Walt > > -----Original Message----- > From: [hidden email] > [mailto:[hidden email]] On Behalf Of UV > Sent: Friday, October 23, 2009 9:03 AM > To: MapGuide Internals Mail List > Subject: [mapguide-internals] RFC60 finalization > > Hi all, > > I looked into the RFC60 code again and added some more features to it > e.g. finding label colors. > > However, a redirection of referenced featureIds which would cause a FDO > lookup seems prohibitive expensive in this context. > I would not like to add this into the code simply as it could increase > delay tile computation a lot. > So the interesting test here are performance related. Simple unit-tests > are not sufficient for this. > Creating complex test data to test performance issues is tedious and > very badly motivated!!! (specification vs. implementation from same hand) > > Secondly, it think it is wrong to request the implementation of the new > stylization for such transparent feature addition. > RFC60 works fine for our case, why should we implement something that > does not harm anything but what we do not need? > This is not thought along open source paradigms. Open source is more > like: Who needs it should code it! > So we should leave this open and the next person that has real test data > which doesn't work can post a defect! > Very easy process! > > > RFC60 is not trivial at all but it solves a real problem in real maps. > To block the inclusion of RFC60 into the code base on those matters is > simply wrong in open source terms as it keeps others from using a new > working feature. > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
|
Jason Birch
|
In reply to this post
by Tom Fukushima
Oh, sorry, I got confused between UV's initial reference to FDO
lookups, and Walts objection about the external symbol definition lookups. Must pay more attention to actual words... On 2009-10-23, Tom Fukushima <[hidden email]> wrote: > I'm trying to understand this...so correct me if I'm wrong. But it seems > that the problem is not with attribute-based stylization. It's that if > someone uses referenced symbols (that is, the LayerDefinition specifies a > resource using a resource ID such as Library://mysymbols/a.SymbolDefinition > instead of embedded XML for a SymbolDefinition in the LayerDefinition) then > a large part of the code will need to be rewritten to support it. > > If the above is the case, I understand that we need to make tradeoffs all > the time, and this may be where we need to make one. I think though that it > would be advantageous for the current developer to do the requested > refactoring now while it is fresh. Later, if defects are found or > refinements (e.g., attribute-based stylization) are done, they could be > addressed at that time---and this would be manageable because wouldn't > require refactoring large parts of the code. > > Could we get some idea of the work that would be required to get support for > the referenced symbol definitions? > > Note that I'm not saying it has to be done (but of course I would prefer it > to be done), I'm trying to get more information on this sticking point (I > believe it is the only one right?) to try to make a judgment. > > Tom > > > -----Original Message----- > From: [hidden email] > [mailto:[hidden email]] On Behalf Of Jason Birch > Sent: Friday, October 23, 2009 9:24 AM > To: MapGuide Internals Mail List > Subject: Re: [mapguide-internals] RFC60 finalization > > It seems to me that we have frequently (too often perhaps?) favoured > expediency over correctness when faced with resourcing or timeline > issues. > > In this case, I'm in favour of placing the burden of change on the > whoever deems that it is critical to support pallette reservation for > attribute-based stylization. > > Jason > > On 2009-10-23, Walt Welton-Lair <[hidden email]> wrote: >>> Secondly, it think it is wrong to request the implementation of the new >>> stylization for such transparent feature addition. >>> RFC60 works fine for our case, why should we implement something that >>> does not harm anything but what we do not need? >> >> The way your code is designed - where it does the work of grabbing the >> colors inside the MdfModel project - WILL NOT WORK for enhanced >> stylization >> in which there are referenced symbol definitions. It's not possible to >> load >> referenced symbols from the resource service from inside the MdfModel >> code. >> This is not a minor defect. It requires that a lot of your code be >> refactored. I already explained the problem last March. Here's the >> excerpt: >> >> "With the new enhanced stylization (RFC 14) the layer definition can >> reference symbol definitions (in addition to inlining them). The symbols, >> of course, define colors which need to be accounted for. Accessing >> referenced symbol definitions requires the resource service - we need to >> get >> the symbol definition resource from the service. The MdfModel project >> (where VectorLayerDefinition is stored) does not have access (nor want >> access) to the resource service. So if you want to properly support the >> new >> enhanced stylization with your RFC (you should) then we'll probably have >> to >> move this ComputeUsedColors method somewhere else. There's a method - >> MgMappingUtil::ComputeStylizationOffset - which does something analogous >> to >> ComputeUsedColors, so possibly we can add ComputeUsedColors to >> MgMappingUtil." >> >> So even if you don't add code to actually parse colors for enhanced style >> layers, you need to keep the bigger picture in mind and design your new >> code >> so it will be easy to add this support. >> >> All along that's the real reason why I wanted you to take a look at this. >> So that you would come to realize that your current design doesn't fully >> work as is. >> >> >>> Open source is more like: Who needs it should code it! >> >> True, but it's not an excuse for not doing the right thing. People can't >> just add anything they want without taking account the bigger picture. >> That's my take on this. >> >> >> Walt >> >> -----Original Message----- >> From: [hidden email] >> [mailto:[hidden email]] On Behalf Of UV >> Sent: Friday, October 23, 2009 9:03 AM >> To: MapGuide Internals Mail List >> Subject: [mapguide-internals] RFC60 finalization >> >> Hi all, >> >> I looked into the RFC60 code again and added some more features to it >> e.g. finding label colors. >> >> However, a redirection of referenced featureIds which would cause a FDO >> lookup seems prohibitive expensive in this context. >> I would not like to add this into the code simply as it could increase >> delay tile computation a lot. >> So the interesting test here are performance related. Simple unit-tests >> are not sufficient for this. >> Creating complex test data to test performance issues is tedious and >> very badly motivated!!! (specification vs. implementation from same hand) >> >> Secondly, it think it is wrong to request the implementation of the new >> stylization for such transparent feature addition. >> RFC60 works fine for our case, why should we implement something that >> does not harm anything but what we do not need? >> This is not thought along open source paradigms. Open source is more >> like: Who needs it should code it! >> So we should leave this open and the next person that has real test data >> which doesn't work can post a defect! >> Very easy process! >> >> >> RFC60 is not trivial at all but it solves a real problem in real maps. >> To block the inclusion of RFC60 into the code base on those matters is >> simply wrong in open source terms as it keeps others from using a new >> working feature. >> _______________________________________________ >> mapguide-internals mailing list >> [hidden email] >> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >> _______________________________________________ >> mapguide-internals mailing list >> [hidden email] >> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >> > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
|
UV-2
|
In reply to this post
by Tom Fukushima
MOTIVATION
The important question is how far the color search in the map should go. To solve the problem of our client we were only looking at areas and line styles. Those are the relevant use cases as they are easily spotted on bordering tiles! (and it is not so computational expensive) To include all the symbolization data including the referenced symbols seems like an computational overkill with no justification. I am not sure if this should be enabled by default! Dont forget that we are only tuning a color quantization algorithm for the paletted PNG8 /GIF tiles with this information!! Its only a gradual improvement of color correctness with limited impact. Tom Fukushima wrote: > I'm trying to understand this...so correct me if I'm wrong. But it seems that the problem is not with attribute-based stylization. It's that if someone uses referenced symbols (that is, the LayerDefinition specifies a resource using a resource ID such as Library://mysymbols/a.SymbolDefinition instead of embedded XML for a SymbolDefinition in the LayerDefinition) then a large part of the code will need to be rewritten to support it. > Refactoring is not really necessary. The access points are very limited. The string collection is capable of dealing with variables and references. Only adding functionality for the bits we did not need and did not implement! This is why I don't think this is my responsibility to do this without funding, no good test cases (maps), and no client. > If the above is the case, I understand that we need to make tradeoffs all the time, and this may be where we need to make one. I think though that it would be advantageous for the current developer to do the requested refactoring now while it is fresh. Later, if defects are found or refinements (e.g., attribute-based stylization) are done, they could be addressed at that time---and this would be manageable because wouldn't require refactoring large parts of the code. > > Could we get some idea of the work that would be required to get support for the referenced symbol definitions? > > Note that I'm not saying it has to be done (but of course I would prefer it to be done), I'm trying to get more information on this sticking point (I believe it is the only one right?) to try to make a judgment. > > Tom > > > -----Original Message----- > From: [hidden email] [mailto:[hidden email]] On Behalf Of Jason Birch > Sent: Friday, October 23, 2009 9:24 AM > To: MapGuide Internals Mail List > Subject: Re: [mapguide-internals] RFC60 finalization > > It seems to me that we have frequently (too often perhaps?) favoured > expediency over correctness when faced with resourcing or timeline > issues. > > In this case, I'm in favour of placing the burden of change on the > whoever deems that it is critical to support pallette reservation for > attribute-based stylization. > > Jason > > On 2009-10-23, Walt Welton-Lair <[hidden email]> wrote: > >>> Secondly, it think it is wrong to request the implementation of the new >>> stylization for such transparent feature addition. >>> RFC60 works fine for our case, why should we implement something that >>> does not harm anything but what we do not need? >>> >> The way your code is designed - where it does the work of grabbing the >> colors inside the MdfModel project - WILL NOT WORK for enhanced stylization >> in which there are referenced symbol definitions. It's not possible to load >> referenced symbols from the resource service from inside the MdfModel code. >> This is not a minor defect. It requires that a lot of your code be >> refactored. I already explained the problem last March. Here's the >> excerpt: >> >> "With the new enhanced stylization (RFC 14) the layer definition can >> reference symbol definitions (in addition to inlining them). The symbols, >> of course, define colors which need to be accounted for. Accessing >> referenced symbol definitions requires the resource service - we need to get >> the symbol definition resource from the service. The MdfModel project >> (where VectorLayerDefinition is stored) does not have access (nor want >> access) to the resource service. So if you want to properly support the new >> enhanced stylization with your RFC (you should) then we'll probably have to >> move this ComputeUsedColors method somewhere else. There's a method - >> MgMappingUtil::ComputeStylizationOffset - which does something analogous to >> ComputeUsedColors, so possibly we can add ComputeUsedColors to >> MgMappingUtil." >> >> So even if you don't add code to actually parse colors for enhanced style >> layers, you need to keep the bigger picture in mind and design your new code >> so it will be easy to add this support. >> >> All along that's the real reason why I wanted you to take a look at this. >> So that you would come to realize that your current design doesn't fully >> work as is. >> >> >> >>> Open source is more like: Who needs it should code it! >>> >> True, but it's not an excuse for not doing the right thing. People can't >> just add anything they want without taking account the bigger picture. >> That's my take on this. >> >> >> Walt >> >> -----Original Message----- >> From: [hidden email] >> [mailto:[hidden email]] On Behalf Of UV >> Sent: Friday, October 23, 2009 9:03 AM >> To: MapGuide Internals Mail List >> Subject: [mapguide-internals] RFC60 finalization >> >> Hi all, >> >> I looked into the RFC60 code again and added some more features to it >> e.g. finding label colors. >> >> However, a redirection of referenced featureIds which would cause a FDO >> lookup seems prohibitive expensive in this context. >> I would not like to add this into the code simply as it could increase >> delay tile computation a lot. >> So the interesting test here are performance related. Simple unit-tests >> are not sufficient for this. >> Creating complex test data to test performance issues is tedious and >> very badly motivated!!! (specification vs. implementation from same hand) >> >> Secondly, it think it is wrong to request the implementation of the new >> stylization for such transparent feature addition. >> RFC60 works fine for our case, why should we implement something that >> does not harm anything but what we do not need? >> This is not thought along open source paradigms. Open source is more >> like: Who needs it should code it! >> So we should leave this open and the next person that has real test data >> which doesn't work can post a defect! >> Very easy process! >> >> >> RFC60 is not trivial at all but it solves a real problem in real maps. >> To block the inclusion of RFC60 into the code base on those matters is >> simply wrong in open source terms as it keeps others from using a new >> working feature. >> _______________________________________________ >> mapguide-internals mailing list >> [hidden email] >> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >> _______________________________________________ >> mapguide-internals mailing list >> [hidden email] >> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >> >> > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > > _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
|
UV-2
|
USAGE of SEMgSymbolManager????
Trying to understand the current implementation and usage of the SEMgSymbolManager some questions occur. Why is there an instance for each thread? In my current understanding there is a map object which could be cached previously. Upon reception of the gettile request this get transformed into a runtime map. So in a very typical GetTile client use case we have a number of parallel neighboring tiles requested from the same map and scalerange. What is the reason to not keep a shared instance of the map object between the threads instead of repeating the whole kaboodle... Symbol manager instances..... map parsing etc. this seems like a read only data structure which can be created once and then be made accessible to all threads via a singleton.... what am I overlooking here? I have a hard time seeing the benefit of caching stylization objects when the cache is created per request/ There could be a lot of potential regarding computational expense if we can leverage the server code to share more of those RO objects between the threads. Instead of creating a cache instance inside the rendering method of the rendering service we could have a global cache structure managing maps and related scaleranges with stylization info including all referenced resources..... any ideas? UV wrote: > MOTIVATION > The important question is how far the color search in the map should go. > To solve the problem of our client we were only looking at areas and > line styles. > Those are the relevant use cases as they are easily spotted on > bordering tiles! > (and it is not so computational expensive) > > To include all the symbolization data including the referenced symbols > seems like an computational overkill with no justification. > I am not sure if this should be enabled by default! > > Dont forget that we are only tuning a color quantization algorithm for > the paletted PNG8 /GIF tiles with this information!! > Its only a gradual improvement of color correctness with limited impact. > > > Tom Fukushima wrote: >> I'm trying to understand this...so correct me if I'm wrong. But it seems that the problem is not with attribute-based stylization. It's that if someone uses referenced symbols (that is, the LayerDefinition specifies a resource using a resource ID such as Library://mysymbols/a.SymbolDefinition instead of embedded XML for a SymbolDefinition in the LayerDefinition) then a large part of the code will need to be rewritten to support it. >> > Refactoring is not really necessary. The access points are very > limited. The string collection is capable of dealing with variables > and references. > Only adding functionality for the bits we did not need and did not > implement! > This is why I don't think this is my responsibility to do this without > funding, no good test cases (maps), and no client. >> If the above is the case, I understand that we need to make tradeoffs all the time, and this may be where we need to make one. I think though that it would be advantageous for the current developer to do the requested refactoring now while it is fresh. Later, if defects are found or refinements (e.g., attribute-based stylization) are done, they could be addressed at that time---and this would be manageable because wouldn't require refactoring large parts of the code. >> >> Could we get some idea of the work that would be required to get support for the referenced symbol definitions? >> >> Note that I'm not saying it has to be done (but of course I would prefer it to be done), I'm trying to get more information on this sticking point (I believe it is the only one right?) to try to make a judgment. >> >> Tom >> >> >> -----Original Message----- >> From: [hidden email] [mailto:[hidden email]] On Behalf Of Jason Birch >> Sent: Friday, October 23, 2009 9:24 AM >> To: MapGuide Internals Mail List >> Subject: Re: [mapguide-internals] RFC60 finalization >> >> It seems to me that we have frequently (too often perhaps?) favoured >> expediency over correctness when faced with resourcing or timeline >> issues. >> >> In this case, I'm in favour of placing the burden of change on the >> whoever deems that it is critical to support pallette reservation for >> attribute-based stylization. >> >> Jason >> >> On 2009-10-23, Walt Welton-Lair <[hidden email]> wrote: >> >>>> Secondly, it think it is wrong to request the implementation of the new >>>> stylization for such transparent feature addition. >>>> RFC60 works fine for our case, why should we implement something that >>>> does not harm anything but what we do not need? >>>> >>> The way your code is designed - where it does the work of grabbing the >>> colors inside the MdfModel project - WILL NOT WORK for enhanced stylization >>> in which there are referenced symbol definitions. It's not possible to load >>> referenced symbols from the resource service from inside the MdfModel code. >>> This is not a minor defect. It requires that a lot of your code be >>> refactored. I already explained the problem last March. Here's the >>> excerpt: >>> >>> "With the new enhanced stylization (RFC 14) the layer definition can >>> reference symbol definitions (in addition to inlining them). The symbols, >>> of course, define colors which need to be accounted for. Accessing >>> referenced symbol definitions requires the resource service - we need to get >>> the symbol definition resource from the service. The MdfModel project >>> (where VectorLayerDefinition is stored) does not have access (nor want >>> access) to the resource service. So if you want to properly support the new >>> enhanced stylization with your RFC (you should) then we'll probably have to >>> move this ComputeUsedColors method somewhere else. There's a method - >>> MgMappingUtil::ComputeStylizationOffset - which does something analogous to >>> ComputeUsedColors, so possibly we can add ComputeUsedColors to >>> MgMappingUtil." >>> >>> So even if you don't add code to actually parse colors for enhanced style >>> layers, you need to keep the bigger picture in mind and design your new code >>> so it will be easy to add this support. >>> >>> All along that's the real reason why I wanted you to take a look at this. >>> So that you would come to realize that your current design doesn't fully >>> work as is. >>> >>> >>> >>>> Open source is more like: Who needs it should code it! >>>> >>> True, but it's not an excuse for not doing the right thing. People can't >>> just add anything they want without taking account the bigger picture. >>> That's my take on this. >>> >>> >>> Walt >>> >>> -----Original Message----- >>> From: [hidden email] >>> [mailto:[hidden email]] On Behalf Of UV >>> Sent: Friday, October 23, 2009 9:03 AM >>> To: MapGuide Internals Mail List >>> Subject: [mapguide-internals] RFC60 finalization >>> >>> Hi all, >>> >>> I looked into the RFC60 code again and added some more features to it >>> e.g. finding label colors. >>> >>> However, a redirection of referenced featureIds which would cause a FDO >>> lookup seems prohibitive expensive in this context. >>> I would not like to add this into the code simply as it could increase >>> delay tile computation a lot. >>> So the interesting test here are performance related. Simple unit-tests >>> are not sufficient for this. >>> Creating complex test data to test performance issues is tedious and >>> very badly motivated!!! (specification vs. implementation from same hand) >>> >>> Secondly, it think it is wrong to request the implementation of the new >>> stylization for such transparent feature addition. >>> RFC60 works fine for our case, why should we implement something that >>> does not harm anything but what we do not need? >>> This is not thought along open source paradigms. Open source is more >>> like: Who needs it should code it! >>> So we should leave this open and the next person that has real test data >>> which doesn't work can post a defect! >>> Very easy process! >>> >>> >>> RFC60 is not trivial at all but it solves a real problem in real maps. >>> To block the inclusion of RFC60 into the code base on those matters is >>> simply wrong in open source terms as it keeps others from using a new >>> working feature. >>> _______________________________________________ >>> mapguide-internals mailing list >>> [hidden email] >>> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >>> _______________________________________________ >>> mapguide-internals mailing list >>> [hidden email] >>> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >>> >>> >> _______________________________________________ >> mapguide-internals mailing list >> [hidden email] >> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >> _______________________________________________ >> mapguide-internals mailing list >> [hidden email] >> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >> >> > _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
Traian Stanev
|
It's implementing the simplest effective way of caching -- generally you have lots of features sharing the same style within a request and it will only parse the style once for those features. Having a global cache is more complicated because it also has to monitor changes to symbol resources -- and it would have been even more annoying to implement back when we theoretically allowed for the resource service to run on a separate machine than the mapping service. Do you have any empirical results of a real world map showing that the current implementation results in a high relative overhead of symbol parsing compared to, say, FDO requests, or PNG saving or the symbol stylization itself? Traian > -----Original Message----- > From: [hidden email] [mailto:mapguide- > [hidden email]] On Behalf Of UV > Sent: Monday, October 26, 2009 8:21 AM > To: MapGuide Internals Mail List > Subject: Re: [mapguide-internals] RFC60 finalization > > USAGE of SEMgSymbolManager???? > > Trying to understand the current implementation and usage of the > SEMgSymbolManager some questions occur. > > Why is there an instance for each thread? > > In my current understanding there is a map object which could be cached > previously. > Upon reception of the gettile request this get transformed into a > runtime map. > > So in a very typical GetTile client use case we have a number of > parallel neighboring tiles requested from the same map and scalerange. > > What is the reason to not keep a shared instance of the map object > between the threads instead of repeating the whole kaboodle... > Symbol manager instances..... map parsing etc. this seems like a read > only data structure which can be created once and then be made > accessible to all threads via a singleton.... > > what am I overlooking here? I have a hard time seeing the benefit of > caching stylization objects when the cache is created per request/ > There could be a lot of potential regarding computational expense if > we > can leverage the server code to share more of those RO objects between > the threads. > > Instead of creating a cache instance inside the rendering method of the > rendering service we could have a global cache structure managing > maps and related scaleranges with stylization info including all > referenced resources..... > > any ideas? > > > UV wrote: > > MOTIVATION > > The important question is how far the color search in the map should > go. > > To solve the problem of our client we were only looking at areas and > > line styles. > > Those are the relevant use cases as they are easily spotted on > > bordering tiles! > > (and it is not so computational expensive) > > > > To include all the symbolization data including the referenced > symbols > > seems like an computational overkill with no justification. > > I am not sure if this should be enabled by default! > > > > Dont forget that we are only tuning a color quantization algorithm > for > > the paletted PNG8 /GIF tiles with this information!! > > Its only a gradual improvement of color correctness with limited > impact. > > > > > > Tom Fukushima wrote: > >> I'm trying to understand this...so correct me if I'm wrong. But it > seems that the problem is not with attribute-based stylization. It's > that if someone uses referenced symbols (that is, the LayerDefinition > specifies a resource using a resource ID such as > Library://mysymbols/a.SymbolDefinition instead of embedded XML for a > SymbolDefinition in the LayerDefinition) then a large part of the code > will need to be rewritten to support it. > >> > > Refactoring is not really necessary. The access points are very > > limited. The string collection is capable of dealing with variables > > and references. > > Only adding functionality for the bits we did not need and did not > > implement! > > This is why I don't think this is my responsibility to do this > without > > funding, no good test cases (maps), and no client. > >> If the above is the case, I understand that we need to make > tradeoffs all the time, and this may be where we need to make one. I > think though that it would be advantageous for the current developer to > do the requested refactoring now while it is fresh. Later, if defects > are found or refinements (e.g., attribute-based stylization) are done, > they could be addressed at that time---and this would be manageable > because wouldn't require refactoring large parts of the code. > >> > >> Could we get some idea of the work that would be required to get > support for the referenced symbol definitions? > >> > >> Note that I'm not saying it has to be done (but of course I would > prefer it to be done), I'm trying to get more information on this > sticking point (I believe it is the only one right?) to try to make a > judgment. > >> > >> Tom > >> > >> > >> -----Original Message----- > >> From: [hidden email] [mailto:mapguide- > [hidden email]] On Behalf Of Jason Birch > >> Sent: Friday, October 23, 2009 9:24 AM > >> To: MapGuide Internals Mail List > >> Subject: Re: [mapguide-internals] RFC60 finalization > >> > >> It seems to me that we have frequently (too often perhaps?) favoured > >> expediency over correctness when faced with resourcing or timeline > >> issues. > >> > >> In this case, I'm in favour of placing the burden of change on the > >> whoever deems that it is critical to support pallette reservation > for > >> attribute-based stylization. > >> > >> Jason > >> > >> On 2009-10-23, Walt Welton-Lair <[hidden email]> > wrote: > >> > >>>> Secondly, it think it is wrong to request the implementation of > the new > >>>> stylization for such transparent feature addition. > >>>> RFC60 works fine for our case, why should we implement something > that > >>>> does not harm anything but what we do not need? > >>>> > >>> The way your code is designed - where it does the work of grabbing > the > >>> colors inside the MdfModel project - WILL NOT WORK for enhanced > stylization > >>> in which there are referenced symbol definitions. It's not > possible to load > >>> referenced symbols from the resource service from inside the > MdfModel code. > >>> This is not a minor defect. It requires that a lot of your code be > >>> refactored. I already explained the problem last March. Here's > the > >>> excerpt: > >>> > >>> "With the new enhanced stylization (RFC 14) the layer definition > can > >>> reference symbol definitions (in addition to inlining them). The > symbols, > >>> of course, define colors which need to be accounted for. Accessing > >>> referenced symbol definitions requires the resource service - we > need to get > >>> the symbol definition resource from the service. The MdfModel > project > >>> (where VectorLayerDefinition is stored) does not have access (nor > want > >>> access) to the resource service. So if you want to properly > support the new > >>> enhanced stylization with your RFC (you should) then we'll probably > have to > >>> move this ComputeUsedColors method somewhere else. There's a > method - > >>> MgMappingUtil::ComputeStylizationOffset - which does something > analogous to > >>> ComputeUsedColors, so possibly we can add ComputeUsedColors to > >>> MgMappingUtil." > >>> > >>> So even if you don't add code to actually parse colors for enhanced > style > >>> layers, you need to keep the bigger picture in mind and design your > new code > >>> so it will be easy to add this support. > >>> > >>> All along that's the real reason why I wanted you to take a look at > this. > >>> So that you would come to realize that your current design doesn't > fully > >>> work as is. > >>> > >>> > >>> > >>>> Open source is more like: Who needs it should code it! > >>>> > >>> True, but it's not an excuse for not doing the right thing. People > can't > >>> just add anything they want without taking account the bigger > picture. > >>> That's my take on this. > >>> > >>> > >>> Walt > >>> > >>> -----Original Message----- > >>> From: [hidden email] > >>> [mailto:[hidden email]] On Behalf Of UV > >>> Sent: Friday, October 23, 2009 9:03 AM > >>> To: MapGuide Internals Mail List > >>> Subject: [mapguide-internals] RFC60 finalization > >>> > >>> Hi all, > >>> > >>> I looked into the RFC60 code again and added some more features to > it > >>> e.g. finding label colors. > >>> > >>> However, a redirection of referenced featureIds which would cause a > FDO > >>> lookup seems prohibitive expensive in this context. > >>> I would not like to add this into the code simply as it could > increase > >>> delay tile computation a lot. > >>> So the interesting test here are performance related. Simple unit- > tests > >>> are not sufficient for this. > >>> Creating complex test data to test performance issues is tedious > and > >>> very badly motivated!!! (specification vs. implementation from same > hand) > >>> > >>> Secondly, it think it is wrong to request the implementation of the > new > >>> stylization for such transparent feature addition. > >>> RFC60 works fine for our case, why should we implement something > that > >>> does not harm anything but what we do not need? > >>> This is not thought along open source paradigms. Open source is > more > >>> like: Who needs it should code it! > >>> So we should leave this open and the next person that has real test > data > >>> which doesn't work can post a defect! > >>> Very easy process! > >>> > >>> > >>> RFC60 is not trivial at all but it solves a real problem in real > maps. > >>> To block the inclusion of RFC60 into the code base on those matters > is > >>> simply wrong in open source terms as it keeps others from using a > new > >>> working feature. > >>> _______________________________________________ > >>> mapguide-internals mailing list > >>> [hidden email] > >>> http://lists.osgeo.org/mailman/listinfo/mapguide-internals > >>> _______________________________________________ > >>> mapguide-internals mailing list > >>> [hidden email] > >>> http://lists.osgeo.org/mailman/listinfo/mapguide-internals > >>> > >>> > >> _______________________________________________ > >> mapguide-internals mailing list > >> [hidden email] > >> http://lists.osgeo.org/mailman/listinfo/mapguide-internals > >> _______________________________________________ > >> mapguide-internals mailing list > >> [hidden email] > >> http://lists.osgeo.org/mailman/listinfo/mapguide-internals > >> > >> > > > > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
|
Walt Welton-Lair
|
In reply to this post
by Walt Welton-Lair
BTW, UV, please understand that my feedback is just part of the normal review process. Your submission will be fairly big and touches some sensitive areas of the code, so you should expect this level of feedback.
For now I think just doing the refactoring would be sufficient so we can move towards the final / formal code review. Enhanced stylization support can be added separately. Walt -----Original Message----- From: [hidden email] [mailto:[hidden email]] On Behalf Of Walt Welton-Lair Sent: Friday, October 23, 2009 12:32 PM To: MapGuide Internals Mail List Subject: RE: [mapguide-internals] RFC60 finalization Yes, my main concern is not with attribute-based stylization. I want the current code to be updated so that it can properly accommodate enhanced stylization going forward. The refactoring work: * Move changes that were done in MdfModel to collect colors into a method in MgMappingUtil. For now that method will only collect colors for the legacy stylization. To add support for enhanced stylization (inlined symbols): * add code to the above method which iterates over the symbol definition object model and collects colors * not a huge task - roughly 1-2 days work is my estimate. To add support for enhanced stylization (referenced symbols): * Assuming inlined symbol support is present, update the method to make use of the SEMgSymbolManager class (in MappingService) to load referenced symbols. Once loaded, pass the symbol to the code which extracts colors for a symbol definition (added during the previous task). * relatively straightforward task - 1 day is sufficient BTW, even if we completely ignore enhanced stylization the current RFC 60 implementation has the same problem when it comes to point layers styled using a symbol library. The layer definition references a DWF symbol library resource and the name of the symbol to use. The current RFC 60 code will be unable to load the DWF symbol and take into account any colors defined in the DWF symbol. But after the refactoring, this would now be possible. Similar to the enhanced stylization, the color collecting code could be updated to use of the RSMgSymbolManager class to load referenced DWF symbols. And of course there would need to be a method to parse a DWF symbol and collect its colors. For both of these, if we do the refactoring then the color collecting code is ready for future enhancements to support missing cases like the above. Walt -----Original Message----- From: [hidden email] [mailto:[hidden email]] On Behalf Of Tom Fukushima Sent: Friday, October 23, 2009 11:56 AM To: MapGuide Internals Mail List Subject: RE: [mapguide-internals] RFC60 finalization I'm trying to understand this...so correct me if I'm wrong. But it seems that the problem is not with attribute-based stylization. It's that if someone uses referenced symbols (that is, the LayerDefinition specifies a resource using a resource ID such as Library://mysymbols/a.SymbolDefinition instead of embedded XML for a SymbolDefinition in the LayerDefinition) then a large part of the code will need to be rewritten to support it. If the above is the case, I understand that we need to make tradeoffs all the time, and this may be where we need to make one. I think though that it would be advantageous for the current developer to do the requested refactoring now while it is fresh. Later, if defects are found or refinements (e.g., attribute-based stylization) are done, they could be addressed at that time---and this would be manageable because wouldn't require refactoring large parts of the code. Could we get some idea of the work that would be required to get support for the referenced symbol definitions? Note that I'm not saying it has to be done (but of course I would prefer it to be done), I'm trying to get more information on this sticking point (I believe it is the only one right?) to try to make a judgment. Tom -----Original Message----- From: [hidden email] [mailto:[hidden email]] On Behalf Of Jason Birch Sent: Friday, October 23, 2009 9:24 AM To: MapGuide Internals Mail List Subject: Re: [mapguide-internals] RFC60 finalization It seems to me that we have frequently (too often perhaps?) favoured expediency over correctness when faced with resourcing or timeline issues. In this case, I'm in favour of placing the burden of change on the whoever deems that it is critical to support pallette reservation for attribute-based stylization. Jason On 2009-10-23, Walt Welton-Lair <[hidden email]> wrote: >> Secondly, it think it is wrong to request the implementation of the new >> stylization for such transparent feature addition. >> RFC60 works fine for our case, why should we implement something that >> does not harm anything but what we do not need? > > The way your code is designed - where it does the work of grabbing the > colors inside the MdfModel project - WILL NOT WORK for enhanced stylization > in which there are referenced symbol definitions. It's not possible to load > referenced symbols from the resource service from inside the MdfModel code. > This is not a minor defect. It requires that a lot of your code be > refactored. I already explained the problem last March. Here's the > excerpt: > > "With the new enhanced stylization (RFC 14) the layer definition can > reference symbol definitions (in addition to inlining them). The symbols, > of course, define colors which need to be accounted for. Accessing > referenced symbol definitions requires the resource service - we need to get > the symbol definition resource from the service. The MdfModel project > (where VectorLayerDefinition is stored) does not have access (nor want > access) to the resource service. So if you want to properly support the new > enhanced stylization with your RFC (you should) then we'll probably have to > move this ComputeUsedColors method somewhere else. There's a method - > MgMappingUtil::ComputeStylizationOffset - which does something analogous to > ComputeUsedColors, so possibly we can add ComputeUsedColors to > MgMappingUtil." > > So even if you don't add code to actually parse colors for enhanced style > layers, you need to keep the bigger picture in mind and design your new code > so it will be easy to add this support. > > All along that's the real reason why I wanted you to take a look at this. > So that you would come to realize that your current design doesn't fully > work as is. > > >> Open source is more like: Who needs it should code it! > > True, but it's not an excuse for not doing the right thing. People can't > just add anything they want without taking account the bigger picture. > That's my take on this. > > > Walt > > -----Original Message----- > From: [hidden email] > [mailto:[hidden email]] On Behalf Of UV > Sent: Friday, October 23, 2009 9:03 AM > To: MapGuide Internals Mail List > Subject: [mapguide-internals] RFC60 finalization > > Hi all, > > I looked into the RFC60 code again and added some more features to it > e.g. finding label colors. > > However, a redirection of referenced featureIds which would cause a FDO > lookup seems prohibitive expensive in this context. > I would not like to add this into the code simply as it could increase > delay tile computation a lot. > So the interesting test here are performance related. Simple unit-tests > are not sufficient for this. > Creating complex test data to test performance issues is tedious and > very badly motivated!!! (specification vs. implementation from same hand) > > Secondly, it think it is wrong to request the implementation of the new > stylization for such transparent feature addition. > RFC60 works fine for our case, why should we implement something that > does not harm anything but what we do not need? > This is not thought along open source paradigms. Open source is more > like: Who needs it should code it! > So we should leave this open and the next person that has real test data > which doesn't work can post a defect! > Very easy process! > > > RFC60 is not trivial at all but it solves a real problem in real maps. > To block the inclusion of RFC60 into the code base on those matters is > simply wrong in open source terms as it keeps others from using a new > working feature. > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
|
UV-2
|
In reply to this post
by Traian Stanev
inline...
Traian Stanev wrote: > It's implementing the simplest effective way of caching -- generally you have lots of features sharing the same style within a request and it will only parse the style once for those features. Having a global cache is more complicated because it also has to monitor changes to symbol resources -- and it would have been even more annoying to implement back when we theoretically allowed for the resource service to run on a separate machine than the mapping service. > ... has the plan to move the resource service to another node been abandonded? If so this opens the route to more efficient use of the internal map structures .... How big is the share of TileRequests in the general picture? Maybe this is a good time to ask people for their usage patterns to collect some statistical usage data.... However, parallel tile requests to the same scalerange of the same map seem like a very popular use case ... ... and they are all sharing identical featuretypestyle data thus I see a lot of potential to streamline the map cache structures by tying it more closely to the map data. In the context of RFC60 the reuse of the symbolcache for the colorsearch requires some code changes due to the current scoping of the symbolcache. A singleton based shared mapobject providing cache structures tied to the maps lifecycle could be a much more performant approach here. Sharing mapdata across http requests this way could significantly improve server performance. Any thoughts?? > Do you have any empirical results of a real world map showing that the current implementation results in a high relative overhead of symbol parsing compared to, say, FDO requests, or PNG saving or the symbol stylization itself? Empirically we only know so far that 4 times a small tile takes A LOT longer than a tile 4 times bigger ..... _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
|
Walt Welton-Lair
|
> Empirically we only know so far that 4 times a small tile takes A LOT
> longer than a tile 4 times bigger ..... And that has nothing to do with having one instance of SEMgSymbolManager per thread, which was your original concern. -----Original Message----- From: [hidden email] [mailto:[hidden email]] On Behalf Of UV Sent: Monday, October 26, 2009 6:27 PM To: MapGuide Internals Mail List Subject: Re: [mapguide-internals] RFC60 finalization inline... Traian Stanev wrote: > It's implementing the simplest effective way of caching -- generally you have lots of features sharing the same style within a request and it will only parse the style once for those features. Having a global cache is more complicated because it also has to monitor changes to symbol resources -- and it would have been even more annoying to implement back when we theoretically allowed for the resource service to run on a separate machine than the mapping service. > ... has the plan to move the resource service to another node been abandonded? If so this opens the route to more efficient use of the internal map structures .... How big is the share of TileRequests in the general picture? Maybe this is a good time to ask people for their usage patterns to collect some statistical usage data.... However, parallel tile requests to the same scalerange of the same map seem like a very popular use case ... ... and they are all sharing identical featuretypestyle data thus I see a lot of potential to streamline the map cache structures by tying it more closely to the map data. In the context of RFC60 the reuse of the symbolcache for the colorsearch requires some code changes due to the current scoping of the symbolcache. A singleton based shared mapobject providing cache structures tied to the maps lifecycle could be a much more performant approach here. Sharing mapdata across http requests this way could significantly improve server performance. Any thoughts?? > Do you have any empirical results of a real world map showing that the current implementation results in a high relative overhead of symbol parsing compared to, say, FDO requests, or PNG saving or the symbol stylization itself? Empirically we only know so far that 4 times a small tile takes A LOT longer than a tile 4 times bigger ..... _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
|
UV-2
|
In reply to this post
by Walt Welton-Lair
I am happy for some constructive comments....
further comments inline Walt Welton-Lair wrote: > BTW, UV, please understand that my feedback is just part of the normal review process. Your submission will be fairly big and touches some sensitive areas of the code, so you should expect this level of feedback. > > For now I think just doing the refactoring would be sufficient so we can move towards the final / formal code review. Enhanced stylization support can be added separately. > > Walt > > -----Original Message----- > From: [hidden email] [mailto:[hidden email]] On Behalf Of Walt Welton-Lair > Sent: Friday, October 23, 2009 12:32 PM > To: MapGuide Internals Mail List > Subject: RE: [mapguide-internals] RFC60 finalization > > Yes, my main concern is not with attribute-based stylization. I want the current code to be updated so that it can properly accommodate enhanced stylization going forward. > > > The refactoring work: > * Move changes that were done in MdfModel to collect colors into a method in MgMappingUtil. For now that method will only collect colors for the legacy stylization. > or even named parameters to the collected string lists is very easy and straightforward.... (structural not pretty code but compact and easy to follow) Those stringlist seem like intuitive properties of the scalerange....why should this be moved to the mappingutil? It is right that the code to interpret the strings belongs there.... > To add support for enhanced stylization (inlined symbols): > * add code to the above method which iterates over the symbol definition object model and collects colors > * not a huge task - roughly 1-2 days work is my estimate. > This is done already! > To add support for enhanced stylization (referenced symbols): > * Assuming inlined symbol support is present, update the method to make use of the SEMgSymbolManager class (in MappingService) to load referenced symbols. Once loaded, pass the symbol to the code which extracts colors for a symbol definition (added during the previous task). > * relatively straightforward task - 1 day is sufficient > there is a scope problem as the symbol manager is not so easy accessible from the code which parses the scaleranges feature type stylization. To make this change I am tempted to extend the lifecycle of the symbolmanager and make it reusable across HTTP requests. > BTW, even if we completely ignore enhanced stylization the current RFC 60 implementation has the same problem when it comes to point layers styled using a symbol library. The layer definition references a DWF symbol library resource and the name of the symbol to use. The current RFC 60 code will be unable to load the DWF symbol and take into account any colors defined in the DWF symbol. But after the refactoring, this would now be possible. Similar to the enhanced stylization, the color collecting code could be updated to use of the RSMgSymbolManager class to load referenced DWF symbols. And of course there would need to be a method to parse a DWF symbol and collect its colors. > > For both of these, if we do the refactoring then the color collecting code is ready for future enhancements to support missing cases like the above. > > > Walt > > the rfc60 code has 3 parts.... 1. collecting the strings from the scalerange FTS during stylize layer and 2. converting the strings into a color table before calling the AGGRenderer... 3. using the colortable to manipulate the color quantization algorithm in the Renderer ...this seems to be flexible and extensible. Do you still think there is a need for refactoring? It seems to me that this separation of tasks addresses the reasoning given..... > -----Original Message----- > From: [hidden email] [mailto:[hidden email]] On Behalf Of Tom Fukushima > Sent: Friday, October 23, 2009 11:56 AM > To: MapGuide Internals Mail List > Subject: RE: [mapguide-internals] RFC60 finalization > > I'm trying to understand this...so correct me if I'm wrong. But it seems that the problem is not with attribute-based stylization. It's that if someone uses referenced symbols (that is, the LayerDefinition specifies a resource using a resource ID such as Library://mysymbols/a.SymbolDefinition instead of embedded XML for a SymbolDefinition in the LayerDefinition) then a large part of the code will need to be rewritten to support it. > > If the above is the case, I understand that we need to make tradeoffs all the time, and this may be where we need to make one. I think though that it would be advantageous for the current developer to do the requested refactoring now while it is fresh. Later, if defects are found or refinements (e.g., attribute-based stylization) are done, they could be addressed at that time---and this would be manageable because wouldn't require refactoring large parts of the code. > > Could we get some idea of the work that would be required to get support for the referenced symbol definitions? > > Note that I'm not saying it has to be done (but of course I would prefer it to be done), I'm trying to get more information on this sticking point (I believe it is the only one right?) to try to make a judgment. > > Tom > > > -----Original Message----- > From: [hidden email] [mailto:[hidden email]] On Behalf Of Jason Birch > Sent: Friday, October 23, 2009 9:24 AM > To: MapGuide Internals Mail List > Subject: Re: [mapguide-internals] RFC60 finalization > > It seems to me that we have frequently (too often perhaps?) favoured > expediency over correctness when faced with resourcing or timeline > issues. > > In this case, I'm in favour of placing the burden of change on the > whoever deems that it is critical to support pallette reservation for > attribute-based stylization. > > Jason > > On 2009-10-23, Walt Welton-Lair <[hidden email]> wrote: > >>> Secondly, it think it is wrong to request the implementation of the new >>> stylization for such transparent feature addition. >>> RFC60 works fine for our case, why should we implement something that >>> does not harm anything but what we do not need? >>> >> The way your code is designed - where it does the work of grabbing the >> colors inside the MdfModel project - WILL NOT WORK for enhanced stylization >> in which there are referenced symbol definitions. It's not possible to load >> referenced symbols from the resource service from inside the MdfModel code. >> This is not a minor defect. It requires that a lot of your code be >> refactored. I already explained the problem last March. Here's the >> excerpt: >> >> "With the new enhanced stylization (RFC 14) the layer definition can >> reference symbol definitions (in addition to inlining them). The symbols, >> of course, define colors which need to be accounted for. Accessing >> referenced symbol definitions requires the resource service - we need to get >> the symbol definition resource from the service. The MdfModel project >> (where VectorLayerDefinition is stored) does not have access (nor want >> access) to the resource service. So if you want to properly support the new >> enhanced stylization with your RFC (you should) then we'll probably have to >> move this ComputeUsedColors method somewhere else. There's a method - >> MgMappingUtil::ComputeStylizationOffset - which does something analogous to >> ComputeUsedColors, so possibly we can add ComputeUsedColors to >> MgMappingUtil." >> >> So even if you don't add code to actually parse colors for enhanced style >> layers, you need to keep the bigger picture in mind and design your new code >> so it will be easy to add this support. >> >> All along that's the real reason why I wanted you to take a look at this. >> So that you would come to realize that your current design doesn't fully >> work as is. >> >> >> >>> Open source is more like: Who needs it should code it! >>> >> True, but it's not an excuse for not doing the right thing. People can't >> just add anything they want without taking account the bigger picture. >> That's my take on this. >> >> >> Walt >> >> -----Original Message----- >> From: [hidden email] >> [mailto:[hidden email]] On Behalf Of UV >> Sent: Friday, October 23, 2009 9:03 AM >> To: MapGuide Internals Mail List >> Subject: [mapguide-internals] RFC60 finalization >> >> Hi all, >> >> I looked into the RFC60 code again and added some more features to it >> e.g. finding label colors. >> >> However, a redirection of referenced featureIds which would cause a FDO >> lookup seems prohibitive expensive in this context. >> I would not like to add this into the code simply as it could increase >> delay tile computation a lot. >> So the interesting test here are performance related. Simple unit-tests >> are not sufficient for this. >> Creating complex test data to test performance issues is tedious and >> very badly motivated!!! (specification vs. implementation from same hand) >> >> Secondly, it think it is wrong to request the implementation of the new >> stylization for such transparent feature addition. >> RFC60 works fine for our case, why should we implement something that >> does not harm anything but what we do not need? >> This is not thought along open source paradigms. Open source is more >> like: Who needs it should code it! >> So we should leave this open and the next person that has real test data >> which doesn't work can post a defect! >> Very easy process! >> >> >> RFC60 is not trivial at all but it solves a real problem in real maps. >> To block the inclusion of RFC60 into the code base on those matters is >> simply wrong in open source terms as it keeps others from using a new >> working feature. >> _______________________________________________ >> mapguide-internals mailing list >> [hidden email] >> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >> _______________________________________________ >> mapguide-internals mailing list >> [hidden email] >> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >> >> > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > > _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
|
Traian Stanev
|
In reply to this post
by Walt Welton-Lair
I agree. "Could be", "a lot" and "significantly" mean nothing. If you said something like "25% of request time for my map is spent parsing symbol definitions", then we can talk about fixing it. Profilers do exist. Traian ________________________________________ From: [hidden email] [[hidden email]] On Behalf Of Walt Welton-Lair [[hidden email]] Sent: Monday, October 26, 2009 7:09 PM To: MapGuide Internals Mail List Subject: RE: [mapguide-internals] RFC60 finalization > Empirically we only know so far that 4 times a small tile takes A LOT > longer than a tile 4 times bigger ..... And that has nothing to do with having one instance of SEMgSymbolManager per thread, which was your original concern. -----Original Message----- From: [hidden email] [mailto:[hidden email]] On Behalf Of UV Sent: Monday, October 26, 2009 6:27 PM To: MapGuide Internals Mail List Subject: Re: [mapguide-internals] RFC60 finalization inline... Traian Stanev wrote: > It's implementing the simplest effective way of caching -- generally you have lots of features sharing the same style within a request and it will only parse the style once for those features. Having a global cache is more complicated because it also has to monitor changes to symbol resources -- and it would have been even more annoying to implement back when we theoretically allowed for the resource service to run on a separate machine than the mapping service. > ... has the plan to move the resource service to another node been abandonded? If so this opens the route to more efficient use of the internal map structures .... How big is the share of TileRequests in the general picture? Maybe this is a good time to ask people for their usage patterns to collect some statistical usage data.... However, parallel tile requests to the same scalerange of the same map seem like a very popular use case ... ... and they are all sharing identical featuretypestyle data thus I see a lot of potential to streamline the map cache structures by tying it more closely to the map data. In the context of RFC60 the reuse of the symbolcache for the colorsearch requires some code changes due to the current scoping of the symbolcache. A singleton based shared mapobject providing cache structures tied to the maps lifecycle could be a much more performant approach here. Sharing mapdata across http requests this way could significantly improve server performance. Any thoughts?? > Do you have any empirical results of a real world map showing that the current implementation results in a high relative overhead of symbol parsing compared to, say, FDO requests, or PNG saving or the symbol stylization itself? Empirically we only know so far that 4 times a small tile takes A LOT longer than a tile 4 times bigger ..... _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
|
UV-2
|
frequently executed code has to be optimal.... I think we can all easily
agree that this is straightforward... Any additional symbol lookup which cannot be served straight from the cache is prohibitive. But anyway you mentioned profilers? Its good that they exist. but we have to be able to use them! Some more details maybe? Currently I am tempted to think that improving the symbol cache is easier than doing any measuring..... Traian Stanev wrote: > I agree. "Could be", "a lot" and "significantly" mean nothing. If you said something like "25% of request time for my map is spent parsing symbol definitions", then we can talk about fixing it. Profilers do exist. > > Traian > > ________________________________________ > From: [hidden email] [[hidden email]] On Behalf Of Walt Welton-Lair [[hidden email]] > Sent: Monday, October 26, 2009 7:09 PM > To: MapGuide Internals Mail List > Subject: RE: [mapguide-internals] RFC60 finalization > > >> Empirically we only know so far that 4 times a small tile takes A LOT >> longer than a tile 4 times bigger ..... >> > > And that has nothing to do with having one instance of SEMgSymbolManager per thread, which was your original concern. > > -----Original Message----- > From: [hidden email] [mailto:[hidden email]] On Behalf Of UV > Sent: Monday, October 26, 2009 6:27 PM > To: MapGuide Internals Mail List > Subject: Re: [mapguide-internals] RFC60 finalization > > inline... > Traian Stanev wrote: > >> It's implementing the simplest effective way of caching -- generally you have lots of features sharing the same style within a request and it will only parse the style once for those features. Having a global cache is more complicated because it also has to monitor changes to symbol resources -- and it would have been even more annoying to implement back when we theoretically allowed for the resource service to run on a separate machine than the mapping service. >> >> > ... has the plan to move the resource service to another node been > abandonded? > If so this opens the route to more efficient use of the internal map > structures .... > > How big is the share of TileRequests in the general picture? > Maybe this is a good time to ask people for their usage patterns to > collect some statistical usage data.... > > However, parallel tile requests to the same scalerange of the same map > seem like a very popular use case ... > ... and they are all sharing identical featuretypestyle data > thus I see a lot of potential to streamline the map cache structures by > tying it more closely to the map data. > > In the context of RFC60 the reuse of the symbolcache for the colorsearch > requires some code changes due to the current scoping of the symbolcache. > A singleton based shared mapobject providing cache structures tied to > the maps lifecycle could be a much more performant approach here. > Sharing mapdata across http requests this way could significantly > improve server performance. > > Any thoughts?? > >> Do you have any empirical results of a real world map showing that the current implementation results in a high relative overhead of symbol parsing compared to, say, FDO requests, or PNG saving or the symbol stylization itself? >> > Empirically we only know so far that 4 times a small tile takes A LOT > longer than a tile 4 times bigger ..... > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > > _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
|
Traian Stanev
|
I use the profiler that comes with the Visual Studio Performance Tools. It works well for profiling the MapGuide server and is straightforward to use in sampling mode (which is better than instrumented mode anyway). Given this, it should not take more than 5 minutes to measure the overhead of style parsing if you already have a problematic map: you attach the profiler to the server process, do some map requests that use styles, then detach the profiler and look at the table of sorted function calls it shows as a result. Traian > -----Original Message----- > From: [hidden email] [mailto:mapguide-internals- > [hidden email]] On Behalf Of UV > Sent: Friday, October 30, 2009 2:17 PM > To: MapGuide Internals Mail List > Subject: Re: [mapguide-internals] RFC60 finalization > > frequently executed code has to be optimal.... I think we can all easily > agree that this is straightforward... > Any additional symbol lookup which cannot be served straight from the > cache is prohibitive. > > But anyway you mentioned profilers? > Its good that they exist. but we have to be able to use them! > > Some more details maybe? > > Currently I am tempted to think that improving the symbol cache is > easier than doing any measuring..... > > Traian Stanev wrote: > > I agree. "Could be", "a lot" and "significantly" mean nothing. If you said > something like "25% of request time for my map is spent parsing symbol > definitions", then we can talk about fixing it. Profilers do exist. > > > > Traian > > > > ________________________________________ > > From: [hidden email] [mapguide-internals- > [hidden email]] On Behalf Of Walt Welton-Lair [walt.welton- > [hidden email]] > > Sent: Monday, October 26, 2009 7:09 PM > > To: MapGuide Internals Mail List > > Subject: RE: [mapguide-internals] RFC60 finalization > > > > > >> Empirically we only know so far that 4 times a small tile takes A LOT > >> longer than a tile 4 times bigger ..... > >> > > > > And that has nothing to do with having one instance of SEMgSymbolManager > per thread, which was your original concern. > > > > -----Original Message----- > > From: [hidden email] [mailto:mapguide- > [hidden email]] On Behalf Of UV > > Sent: Monday, October 26, 2009 6:27 PM > > To: MapGuide Internals Mail List > > Subject: Re: [mapguide-internals] RFC60 finalization > > > > inline... > > Traian Stanev wrote: > > > >> It's implementing the simplest effective way of caching -- generally you > have lots of features sharing the same style within a request and it will > only parse the style once for those features. Having a global cache is more > complicated because it also has to monitor changes to symbol resources -- > and it would have been even more annoying to implement back when we > theoretically allowed for the resource service to run on a separate machine > than the mapping service. > >> > >> > > ... has the plan to move the resource service to another node been > > abandonded? > > If so this opens the route to more efficient use of the internal map > > structures .... > > > > How big is the share of TileRequests in the general picture? > > Maybe this is a good time to ask people for their usage patterns to > > collect some statistical usage data.... > > > > However, parallel tile requests to the same scalerange of the same map > > seem like a very popular use case ... > > ... and they are all sharing identical featuretypestyle data > > thus I see a lot of potential to streamline the map cache structures by > > tying it more closely to the map data. > > > > In the context of RFC60 the reuse of the symbolcache for the colorsearch > > requires some code changes due to the current scoping of the symbolcache. > > A singleton based shared mapobject providing cache structures tied to > > the maps lifecycle could be a much more performant approach here. > > Sharing mapdata across http requests this way could significantly > > improve server performance. > > > > Any thoughts?? > > > >> Do you have any empirical results of a real world map showing that the > current implementation results in a high relative overhead of symbol parsing > compared to, say, FDO requests, or PNG saving or the symbol stylization > itself? > >> > > Empirically we only know so far that 4 times a small tile takes A LOT > > longer than a tile 4 times bigger ..... > > _______________________________________________ > > mapguide-internals mailing list > > [hidden email] > > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > > _______________________________________________ > > mapguide-internals mailing list > > [hidden email] > > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > > _______________________________________________ > > mapguide-internals mailing list > > [hidden email] > > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > > > > > > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
|
UV-2
|
Thanks that will be helpful to assess the overhead later....
I consider now moving the color parsing into the stylizer.... That looks like the best place as the same data structures are used here. As the stylizer is pluggable this seems perfect to do comparitive analysis. Walt, any comments? Traian Stanev wrote: > I use the profiler that comes with the Visual Studio Performance Tools. It works well for profiling the MapGuide server and is straightforward to use in sampling mode (which is better than instrumented mode anyway). Given this, it should not take more than 5 minutes to measure the overhead of style parsing if you already have a problematic map: you attach the profiler to the server process, do some map requests that use styles, then detach the profiler and look at the table of sorted function calls it shows as a result. > > > Traian > > > >> -----Original Message----- >> From: [hidden email] [mailto:mapguide-internals- >> [hidden email]] On Behalf Of UV >> Sent: Friday, October 30, 2009 2:17 PM >> To: MapGuide Internals Mail List >> Subject: Re: [mapguide-internals] RFC60 finalization >> >> frequently executed code has to be optimal.... I think we can all easily >> agree that this is straightforward... >> Any additional symbol lookup which cannot be served straight from the >> cache is prohibitive. >> >> But anyway you mentioned profilers? >> Its good that they exist. but we have to be able to use them! >> >> Some more details maybe? >> >> Currently I am tempted to think that improving the symbol cache is >> easier than doing any measuring..... >> >> Traian Stanev wrote: >> >>> I agree. "Could be", "a lot" and "significantly" mean nothing. If you said >>> >> something like "25% of request time for my map is spent parsing symbol >> definitions", then we can talk about fixing it. Profilers do exist. >> >>> Traian >>> >>> ________________________________________ >>> From: [hidden email] [mapguide-internals- >>> >> [hidden email]] On Behalf Of Walt Welton-Lair [walt.welton- >> [hidden email]] >> >>> Sent: Monday, October 26, 2009 7:09 PM >>> To: MapGuide Internals Mail List >>> Subject: RE: [mapguide-internals] RFC60 finalization >>> >>> >>> >>>> Empirically we only know so far that 4 times a small tile takes A LOT >>>> longer than a tile 4 times bigger ..... >>>> >>>> >>> And that has nothing to do with having one instance of SEMgSymbolManager >>> >> per thread, which was your original concern. >> >>> -----Original Message----- >>> From: [hidden email] [mailto:mapguide- >>> >> [hidden email]] On Behalf Of UV >> >>> Sent: Monday, October 26, 2009 6:27 PM >>> To: MapGuide Internals Mail List >>> Subject: Re: [mapguide-internals] RFC60 finalization >>> >>> inline... >>> Traian Stanev wrote: >>> >>> >>>> It's implementing the simplest effective way of caching -- generally you >>>> >> have lots of features sharing the same style within a request and it will >> only parse the style once for those features. Having a global cache is more >> complicated because it also has to monitor changes to symbol resources -- >> and it would have been even more annoying to implement back when we >> theoretically allowed for the resource service to run on a separate machine >> than the mapping service. >> >>>> >>> ... has the plan to move the resource service to another node been >>> abandonded? >>> If so this opens the route to more efficient use of the internal map >>> structures .... >>> >>> How big is the share of TileRequests in the general picture? >>> Maybe this is a good time to ask people for their usage patterns to >>> collect some statistical usage data.... >>> >>> However, parallel tile requests to the same scalerange of the same map >>> seem like a very popular use case ... >>> ... and they are all sharing identical featuretypestyle data >>> thus I see a lot of potential to streamline the map cache structures by >>> tying it more closely to the map data. >>> >>> In the context of RFC60 the reuse of the symbolcache for the colorsearch >>> requires some code changes due to the current scoping of the symbolcache. >>> A singleton based shared mapobject providing cache structures tied to >>> the maps lifecycle could be a much more performant approach here. >>> Sharing mapdata across http requests this way could significantly >>> improve server performance. >>> >>> Any thoughts?? >>> >>> >>>> Do you have any empirical results of a real world map showing that the >>>> >> current implementation results in a high relative overhead of symbol parsing >> compared to, say, FDO requests, or PNG saving or the symbol stylization >> itself? >> >>> Empirically we only know so far that 4 times a small tile takes A LOT >>> longer than a tile 4 times bigger ..... >>> _______________________________________________ >>> mapguide-internals mailing list >>> [hidden email] >>> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >>> _______________________________________________ >>> mapguide-internals mailing list >>> [hidden email] >>> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >>> _______________________________________________ >>> mapguide-internals mailing list >>> [hidden email] >>> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >>> >>> >>> >> _______________________________________________ >> mapguide-internals mailing list >> [hidden email] >> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >> > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > > _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
|
Walt Welton-Lair
|
Go for it.
-----Original Message----- From: [hidden email] [mailto:[hidden email]] On Behalf Of UV Sent: Friday, October 30, 2009 3:29 PM To: MapGuide Internals Mail List Subject: Re: [mapguide-internals] RFC60 finalization ... Stylizing Thanks that will be helpful to assess the overhead later.... I consider now moving the color parsing into the stylizer.... That looks like the best place as the same data structures are used here. As the stylizer is pluggable this seems perfect to do comparitive analysis. Walt, any comments? Traian Stanev wrote: > I use the profiler that comes with the Visual Studio Performance Tools. It works well for profiling the MapGuide server and is straightforward to use in sampling mode (which is better than instrumented mode anyway). Given this, it should not take more than 5 minutes to measure the overhead of style parsing if you already have a problematic map: you attach the profiler to the server process, do some map requests that use styles, then detach the profiler and look at the table of sorted function calls it shows as a result. > > > Traian > > > >> -----Original Message----- >> From: [hidden email] [mailto:mapguide-internals- >> [hidden email]] On Behalf Of UV >> Sent: Friday, October 30, 2009 2:17 PM >> To: MapGuide Internals Mail List >> Subject: Re: [mapguide-internals] RFC60 finalization >> >> frequently executed code has to be optimal.... I think we can all easily >> agree that this is straightforward... >> Any additional symbol lookup which cannot be served straight from the >> cache is prohibitive. >> >> But anyway you mentioned profilers? >> Its good that they exist. but we have to be able to use them! >> >> Some more details maybe? >> >> Currently I am tempted to think that improving the symbol cache is >> easier than doing any measuring..... >> >> Traian Stanev wrote: >> >>> I agree. "Could be", "a lot" and "significantly" mean nothing. If you said >>> >> something like "25% of request time for my map is spent parsing symbol >> definitions", then we can talk about fixing it. Profilers do exist. >> >>> Traian >>> >>> ________________________________________ >>> From: [hidden email] [mapguide-internals- >>> >> [hidden email]] On Behalf Of Walt Welton-Lair [walt.welton- >> [hidden email]] >> >>> Sent: Monday, October 26, 2009 7:09 PM >>> To: MapGuide Internals Mail List >>> Subject: RE: [mapguide-internals] RFC60 finalization >>> >>> >>> >>>> Empirically we only know so far that 4 times a small tile takes A LOT >>>> longer than a tile 4 times bigger ..... >>>> >>>> >>> And that has nothing to do with having one instance of SEMgSymbolManager >>> >> per thread, which was your original concern. >> >>> -----Original Message----- >>> From: [hidden email] [mailto:mapguide- >>> >> [hidden email]] On Behalf Of UV >> >>> Sent: Monday, October 26, 2009 6:27 PM >>> To: MapGuide Internals Mail List >>> Subject: Re: [mapguide-internals] RFC60 finalization >>> >>> inline... >>> Traian Stanev wrote: >>> >>> >>>> It's implementing the simplest effective way of caching -- generally you >>>> >> have lots of features sharing the same style within a request and it will >> only parse the style once for those features. Having a global cache is more >> complicated because it also has to monitor changes to symbol resources -- >> and it would have been even more annoying to implement back when we >> theoretically allowed for the resource service to run on a separate machine >> than the mapping service. >> >>>> >>> ... has the plan to move the resource service to another node been >>> abandonded? >>> If so this opens the route to more efficient use of the internal map >>> structures .... >>> >>> How big is the share of TileRequests in the general picture? >>> Maybe this is a good time to ask people for their usage patterns to >>> collect some statistical usage data.... >>> >>> However, parallel tile requests to the same scalerange of the same map >>> seem like a very popular use case ... >>> ... and they are all sharing identical featuretypestyle data >>> thus I see a lot of potential to streamline the map cache structures by >>> tying it more closely to the map data. >>> >>> In the context of RFC60 the reuse of the symbolcache for the colorsearch >>> requires some code changes due to the current scoping of the symbolcache. >>> A singleton based shared mapobject providing cache structures tied to >>> the maps lifecycle could be a much more performant approach here. >>> Sharing mapdata across http requests this way could significantly >>> improve server performance. >>> >>> Any thoughts?? >>> >>> >>>> Do you have any empirical results of a real world map showing that the >>>> >> current implementation results in a high relative overhead of symbol parsing >> compared to, say, FDO requests, or PNG saving or the symbol stylization >> itself? >> >>> Empirically we only know so far that 4 times a small tile takes A LOT >>> longer than a tile 4 times bigger ..... >>> _______________________________________________ >>> mapguide-internals mailing list >>> [hidden email] >>> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >>> _______________________________________________ >>> mapguide-internals mailing list >>> [hidden email] >>> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >>> _______________________________________________ >>> mapguide-internals mailing list >>> [hidden email] >>> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >>> >>> >>> >> _______________________________________________ >> mapguide-internals mailing list >> [hidden email] >> http://lists.osgeo.org/mailman/listinfo/mapguide-internals >> > _______________________________________________ > mapguide-internals mailing list > [hidden email] > http://lists.osgeo.org/mailman/listinfo/mapguide-internals > > _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
web
|
Hello all,
Would anyone do a review for the ticket: http://trac.osgeo.org/mapguide/ticket/1021 Because I did a patch that corrects the wrong behaviour in FF when we supply a sessionId in params and FF displays anyway the login box. With my patch it works fine with FF and IE. regard, Rémy _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
Arthur Liu
|
Hi Rémy and other experts,
I added some comments in ticket http://trac.osgeo.org/mapguide/ticket/1021. Would you please review it? Thanks, Arthur -----Original Message----- From: [hidden email] [mailto:[hidden email]] On Behalf Of web Sent: Saturday, October 31, 2009 10:45 PM To: MapGuide Internals Mail List Subject: [mapguide-internals] Review request for ticket:1021, patch submitted. Hello all, Would anyone do a review for the ticket: http://trac.osgeo.org/mapguide/ticket/1021 Because I did a patch that corrects the wrong behaviour in FF when we supply a sessionId in params and FF displays anyway the login box. With my patch it works fine with FF and IE. regard, Rémy _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals _______________________________________________ mapguide-internals mailing list [hidden email] http://lists.osgeo.org/mailman/listinfo/mapguide-internals |
||||||||||||||||
| Free Embeddable Forum Powered by Nabble | Help |
