-
Notifications
You must be signed in to change notification settings - Fork 130
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
Some documentation improvements, add support for logging, OFPortBitMap fixes #108
Conversation
Seems like it might be non-trivial to make it completely/consistently a generation-time option. Presumably that would/could involve converting at least some of the currently pre-written files to be templates that would include/exclude the instrumentation code depending on the generation-time configuration, at least assuming we want/need to instrument any of the pre-written code. What do you think the overhead would be currently with having the code include the instrumentation logic but disabling it with a runtime option? It seems like the simplest thing to do would be to always include the instrumentation code and only use the runtime check, unless we thing that's going to add to much overhead. Do you envision that the instrumentation code would be useful in debugging issues at a customer deployment? If so, how would we enable it in that case if the instrumentation is disabled at generation/compile time? Presumably they'd need to drop in a different jar with the instrumentation code enabled? Or do we think that the instrumentation code would only really be useful for us during development? It isn't clear to me how the current code is designed to be used to enable the instrumentation. Is the idea that the developer would go in and tweak the OFInstrumentationOptions.TRACE_READ value to be true and then recompile openflowj? Presumably the idea there is that the compiler would completely optimize away the dead instrumentation code if TRACE_READ is false? |
I removed the InstrumentationOptions object, now relying on isTraceEnabled() to direct tracing. |
SELF-ACK |
ABAT: START_MERGE: Log file at http://sbs1/abat/2013.11.06.0127-m.loxigen.master/abat.log |
Some documentation improvements, add support for logging, OFPortBitMap fixes (#108)
ABAT: ACCEPT: Successfully merged In case you want to see the build log, check out : |
Reviewer: @vaterlaus
Rob, can you take a look at the logging stuff -- I'm kind of on the fence about different options for the instrumentation (making it generate-time optional (loxi option), copmile-time optional (Insturmentation options) run time optinoal(slf4j.isTraceEnabled()).
Thoughts?