Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to break class declarations up #71

Open
rpatters1 opened this issue Sep 6, 2022 · 9 comments
Open

Ability to break class declarations up #71

rpatters1 opened this issue Sep 6, 2022 · 9 comments
Labels
question Further information is requested

Comments

@rpatters1
Copy link
Contributor

I use refl-cpp to shove a C++ class framework into Lua with LuaBridge. The framework cannot be modified easily. Some of the classes have a large number of property setters and getters. (Around 200 is the max I have, if I recall correctly.) Per the refl-cpp documentation (and in my lived experience), finding the setters and getters on a list that long becomes exponentially difficult for the compiler.

LuaBridge allows classes to be loaded in chunks, so it would be very handy if refl-cpp allowed classes to be chunked as well. Conceptually the syntax might be

REFL_TYPE(mytype)
   REFL_FUNC(func1)
   ...
REFL_END

REFL_TYPE(mytype, continuation())
   REFL_FUNC(funcx1)
   ...
REFL_END

It almost works now, if I split those two into separate cpp files. The only issue is that it counts the member numbers from 1 each time in the template parameters, so the later functions simply get the same functions as the earlier ones. Worst case would be an option to tell you what number to start counting with. But maybe there could be an addition chunk number template parameter. Just thinking out loud here.

That or figure out a way to not have it be so compiler-intensive. I realize this is something of an ask.

@veselink1 veselink1 added the question Further information is requested label Sep 6, 2022
@veselink1
Copy link
Owner

Yeah, refl-cpp doesn't handle large classes very well. Splitting up classes in this way might not be possible due to language limitations. There might be better ways to work around this issue.

Just to clarify, are you saying you have multiple classes with ~200 reflected members (getters and setters included)? And you do some sort of looping over of the members, matching getters and setters, and this is what is causing long compile times? Are you using the get_reader/get_writer API?

@rpatters1
Copy link
Contributor Author

rpatters1 commented Sep 6, 2022

I have several large classes, many around 100 members, but a couple are around 200 members. I loop using the get_reader/get_writer api. You've helped me out before by speeding it up considerably a few months ago.

The chunk idea is motivated by the notion that I could separate the getters and setters into chunks so that get_reader/get_writer would not have to search so many members. (I'm hoping this rewrite could be limited to only the largest classes, to avoid having to refactor the entire reflection code base.)

@rpatters1
Copy link
Contributor Author

Another approach might be to have a way to specify the setter in the macros.

@veselink1
Copy link
Owner

Can you try this PR #72 and see if it improves compile times for your use case? Thanks.

@rpatters1
Copy link
Contributor Author

rpatters1 commented Sep 6, 2022

It may make a slight difference, but it isn't a lot. I just had a different idea of something to try: I could sort the member of the largest classes and then add attributes to all their getters that basically tell them to use n+1 for their setters. Then I could avoid has_writer/has_reader entirely.

Since we last corresponded (several months ago) I have created a script to generate the REFL_FUNC code automatically from the existing Lua environment. Sorting a few of the existing larger classes will not be an issue. (The script can also add attributes, because it's writing a text file with anything I tell it to.)

But I'm thinking this would be less messy code if activated by a type-level attribute. If such a thing exists now, I haven't found it in the docs. (But I easily might not have.) Still, a user-defined class-level attribute would be a cool nice to have. I mean the ability to add an attribute like this:

REFL_TYPE (mytype, fancy_attribute())

I could accomplish the same goal by adding a template bool parameter to my looping routine, but that seems less elegant than a user-defined attribute on REFL_FUNC.

@rpatters1
Copy link
Contributor Author

On further reflection, maybe a setter_is_next attribute on the getter is all I need. Going to try it.

@rpatters1
Copy link
Contributor Author

rpatters1 commented Sep 6, 2022

Hmm. Except in the loop:

   for_each(members, [&](auto member)
   {
      // I have member.
      // how do I get the next member from it?
   });

@veselink1
Copy link
Owner

I could sort the member of the largest classes and then add attributes to all their getters that basically tell them to use n+1 for their setters. [...]
On further reflection, maybe a setter_is_next attribute on the getter is all I need. Going to try it.

This is an optimisation that is already done internally and does make a big difference in my experience. The PR I linked further improves performance.

Each setter must be either right before or right after the corresponding getter, there is no need to specify additional attributes. Definitely worth trying.

// Optimisation for the getter defined after setter pattern.

Just to give an idea of the difference that it makes, Matching getters and setters of a class with 400 methods (200 properties) takes 4s to compile if the properties as sorted and #72 is applied. Pre #72 and in the case of unsorted getters and setters, compilation takes 25s.

https://github.com/veselink1/refl-cpp/blob/master/bench/bench-large-pod-search.cpp

@rpatters1
Copy link
Contributor Author

For context, my Windows machine is severely under-powered. (This is a problem in itself, but since my main dev machine is my Mac, which is lightening-fast, I have been living with it.) All of the following timings are from the under-powered Win machine.

Original compile time for largest class (which is in its own cpp): ~7:00.

This is to confirm that:

  • sorting so that setters follow getters reduces the compile time to ~2:00.
  • applying Skip over 16/64 items per instantiation of get_t #72 further reduces the time to ~40s.
  • directly using member_index + 1 as the setter reduces the compile time to less than 30s. (I gleaned how to do that from get_writer.)

I'm guessing the difference between the last two bullets is primarily because get_writer tries member_index-1 first. If I may be so bold as to suggest, because "Get" comes alphabetically before "Set", I would try member_index+1 first for get_writer and member_index-1 first for get_reader.

Also, the comments are incorrect, fwiw.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants